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

Only use importlib.resources's new files() / Traversable API on Python ≥3.11 #204

Merged
merged 1 commit into from Sep 13, 2022

Conversation

tsibley
Copy link
Contributor

@tsibley tsibley commented Sep 13, 2022

Using importlib.resource's files() API on 3.9 and 3.10 causes a TypeError on 3.9 and a ValueError on 3.10 when running under a third-party meta path importer (like PyOxidizer's OxidizedImporter) that doesn't support the relatively-new API. This is because the full adapter layer (importlib.resources._adapters) for the older importlib resources API doesn't exist until Python 3.11.

The older resources API is now used by 3.7–3.10, as it was prior to the certifi 2022.06.15.1 release. This codepath has existed in certifi since April 2020 (3fc8fec).

An alternative to this change would be testing the actual importer in use at runtime (e.g. certifi.__loader__) for files() support, but that seemed more complex than reverting to the previous codepath here.

Resolves: #203
Related-to: #199
Related-to: #123

…n ≥3.11

Using importlib.resource's files() API on 3.9 and 3.10 causes a
TypeError on 3.9 and a ValueError on 3.10 when running under a
third-party meta path importer (like PyOxidizer's OxidizedImporter) that
doesn't support the relatively-new API.  This is because the full
adapter layer (importlib.resources._adapters) for the older importlib
resources API doesn't exist until Python 3.11.

The older resources API is now used by 3.7–3.10, as it was prior to the
certifi 2022.06.15.1 release.  This codepath has existed in certifi
since April 2020 (3fc8fec).

An alternative to this change would be testing the actual importer in
use at runtime (e.g. certifi.__loader__) for files() support, but that
seemed more complex than reverting to the previous codepath here.

Resolves: certifi#203
Related-to: certifi#199
Related-to: certifi#123
@tsibley
Copy link
Contributor Author

tsibley commented Sep 13, 2022

I haven't run the tests locally yet, so would appreciate approval for the CI workflows to make sure it's all good.

Update: running them on my fork.

@tsibley
Copy link
Contributor Author

tsibley commented Sep 13, 2022

CI on my fork passed. I can also confirm that a minimal PyOxidizer build with this PR yields a usable certifi (and that the same build from current master doesn't).

@sigmavirus24
Copy link
Member

I approved the test run here too

@Lukasa Lukasa merged commit de0eae1 into certifi:master Sep 13, 2022
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.

certifi 2022.6.15.1 broken on Python 3.9 and 3.10 under third-party importers like PyOxidizer
3 participants