1Password / connect-sdk-python

Python SDK for 1Password Connect
https://developer.1password.com/docs/connect
MIT License
200 stars 30 forks source link

Undeclared dependency: six (in generated code) #85

Closed mjpieters closed 8 months ago

mjpieters commented 11 months ago

While looking into the codebase to potentially help out with PRs, I noticed that the onepasswordconnectsdk.models.generator_recipe module includes:

import six

# ...

        for attr, _ in six.iteritems(self.openapi_types):

The import is undeclared as a dependency of the project; the only reason this hasn't been an issue so far is because it is a transitive dependency for python-dateutil. There is no guarantee that all future python-dateutil releases will include six as a dependency.

The import can trivially be removed as connect-sdk-python now requires Python 3.7 and up; the following line would be equivalent, more efficient and removes the need to import six:

        for attr in self.openapi_types:

I note that the specific OpenAPI code generator now only supports Python 3.7 and up, so you could just re-generate the code.

Another option is to use pyupgrade to rewrite the code for you to remove the Python 2.x support altogether, but you'll have to manually remove the import six line in that case, and pyupgrade would rewrite the above to use for attr, _ in self.openapi_types.items():, missing the iteration-over-dictionaries-yields-keys trick (the values are ignored in the loop, so .items() is overkill here).

mjpieters commented 11 months ago

Perhaps regenerating would be a step too far; the new generator adds pydantic as a dependency to enforce type safety and correctness, so could radically alter how the library responds to types that may or may not be acceptable to the SDK now.