1Password / connect-sdk-python

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

Get rid of python2 support #61

Closed volodymyrZotov closed 1 year ago

volodymyrZotov commented 1 year ago

Users ask to get rid of python2 support as it is not possible for them to use this Connect client in new projects.

The changes are done in PR:

The package is functionally tested. No issues were found after applying the changes.

Resolves #59

codecov-commenter commented 1 year ago

Codecov Report

Base: 76.29% // Head: 76.11% // Decreases project coverage by -0.17% :warning:

Coverage data is based on head (3a3ddb6) compared to base (5e70b02). Patch coverage: 14.28% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #61 +/- ## ========================================== - Coverage 76.29% 76.11% -0.18% ========================================== Files 21 21 Lines 1599 1587 -12 ========================================== - Hits 1220 1208 -12 Misses 379 379 ``` | [Impacted Files](https://codecov.io/gh/1Password/connect-sdk-python/pull/61?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/onepasswordconnectsdk/models/error.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvZXJyb3IucHk=) | `40.00% <0.00%> (-1.18%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/field.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvZmllbGQucHk=) | `82.60% <0.00%> (-0.15%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/field\_section.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvZmllbGRfc2VjdGlvbi5weQ==) | `53.65% <0.00%> (-1.11%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/file.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvZmlsZS5weQ==) | `40.90% <0.00%> (-0.89%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/item.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvaXRlbS5weQ==) | `86.62% <0.00%> (-0.09%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/item\_details.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvaXRlbV9kZXRhaWxzLnB5) | `40.00% <0.00%> (-1.18%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/item\_urls.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvaXRlbV91cmxzLnB5) | `40.81% <0.00%> (-1.19%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/item\_vault.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvaXRlbV92YXVsdC5weQ==) | `52.50% <0.00%> (-1.16%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/section.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvc2VjdGlvbi5weQ==) | `61.22% <0.00%> (-0.78%)` | :arrow_down: | | [src/onepasswordconnectsdk/models/summary\_item.py](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL29uZXBhc3N3b3JkY29ubmVjdHNkay9tb2RlbHMvc3VtbWFyeV9pdGVtLnB5) | `78.57% <0.00%> (-0.16%)` | :arrow_down: | | ... and [2 more](https://codecov.io/gh/1Password/connect-sdk-python/pull/61/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

edif2008 commented 1 year ago

This looks good to me. A couple of questions before approving this PR:

Also, one small notice. Since you've removed the six dependency in a couple of files, now we have two new lines between the imports and the code itself. I believe having only one is enough, so you can remove the extra newline as well.

volodymyrZotov commented 1 year ago

@edif2008 Thank you for reviewing!

  1. It works with higher versions. The latest I tried was v3.10.5.
  2. Nothing to clean up right now. We need all of them.
  3. Good question. Don't have an exact answer, unfortunately. The detailed test report says nothing also.

And regarding the code formatting. Pycharm suggest to use new 2 lines between imports section and the rest of the code PEP 8: E302 expected 2 blank lines, found 1. I'd stick with 2, as it was before the changes also.