Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SG-32061 Python3.7+ Syntax #297

Closed
wants to merge 13 commits into from

Conversation

mathbou
Copy link

@mathbou mathbou commented Aug 21, 2023

Hi there,

I just saw through SG-31340-deprecation-py2 that py2 has officially been deprecated, so I updated the API to embrace a py3.7+ syntax

The biggest changes:

  • six and sgsix aren't needed anymore
  • No need for two versions of httplib2 next to each other, we use the PyPI version instead. This also means pyparsing can be removed from the lib.
  • certifi are also fetched from PyPI
  • replace printf syntax with fstrings and str.format

@carlos-villavicencio-adsk
Copy link
Contributor

Thank you for that! Do you know why unit tests are failing? It seems they share the same root cause.

@mathbou
Copy link
Author

mathbou commented Aug 28, 2023

Thank you for that! Do you know why unit tests are failing? It seems they share the same root cause.

I think I found the reason: CI tests relies on variables given by the run_tests.yml file. If tests are triggered from a PR, those value should be null, but the syntax that has been used prevent that, and instead the name of those variables were returned.

@carlos-villavicencio-adsk
Copy link
Contributor

Amazing!

I have one question, since tk-core bundles python-api module, would it be an issue to have dependencies such as certifi not bundled as before?

@carlos-villavicencio-adsk carlos-villavicencio-adsk changed the title Python3.7+ Syntax SG-32061 Python3.7+ Syntax Aug 29, 2023
@mathbou
Copy link
Author

mathbou commented Aug 29, 2023

I have one question, since tk-core bundles python-api module, would it be an issue to have dependencies such as certifi not bundled as before?

Indeed yes, It will not work as is. I made a PR for the tk-core that would support this "vendor-less" version shotgunsoftware/tk-core#914

@carlos-villavicencio-adsk
Copy link
Contributor

Thank you for the clarification. If it's possible, can you split this work into two different PRs? We want to address these improvements separately. So ideally the extraction of the bundled dependencies can be reviewed outside of this scope.

This was referenced Sep 10, 2023
@mathbou
Copy link
Author

mathbou commented Sep 10, 2023

@carlos-villavicencio-adsk

I split this PR in two

  • The syntax is here
  • The vendorless update there

A rebase will probably be required between the two merge to ensure there are no conflict between them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants