Fix deprecation warning on Python 3.11 #193

adamchainz · 2022-05-11T09:29:37Z

Fixes #192.

adamchainz · 2022-05-11T09:30:09Z

I ran tests on Python 3.7+ locally, checking for warnings, with:

[tox]
envlist =
    py{37,38,39,310,311}

[testenv]
commands = python -X dev -W error -m pytest
deps = pytest

None emitted.

Edit: I've also pushed -W error to the GitHub CI setup so warnings can be caught in the future before users experience them.

adamchainz · 2022-05-11T09:45:05Z

Probably #167 should be merged first.

hugovk · 2022-05-19T12:32:49Z

@adamchainz #167 has been merged.

adamchainz · 2022-05-19T17:10:43Z

Rebased!

AA-Turner

3.6 is now EOL, so you could just drop the final fallback & support for 3.6 -- every upstream supported version has a version of importlib.resources.

A

certifi/core.py

sigmavirus24 · 2022-06-08T17:18:21Z

3.6 is now EOL, so you could just drop the final fallback & support for 3.6 -- every upstream supported version has a version of importlib.resources.

A

Not every supported version has the same API in the standard library because it's API is explicitly not stable until maybe 3.10. The fallback is necessary unless we drop everything<3.10 and even then there's a commitment to moving the API quickly to something "better" if something seemingly better is identified without concern for stability promises

AA-Turner · 2022-06-08T17:21:03Z

Sorry, by the final fallback I meant the __file__ version -- the path API will remain stable in Python 3.7 and 3.8, I believe?

A

sigmavirus24 · 2022-06-09T08:51:17Z

Sorry, by the final fallback I meant the __file__ version -- the path API will remain stable in Python 3.7 and 3.8, I believe?

A

It changed after that, and may change in 3.12, 3.13 or some other future version

adamchainz · 2022-06-11T09:29:59Z

Okay, I've dropped Python 3.6 support, added the 3.11 tag, and removed the Python <3.7 branch.

setup.py

AA-Turner

This suggests inlining contents() into the version checks using the native importlib.resources APIs, rather than coercing to a filesystem path.

I can't put a GH suggestion to delete the top level contents() as it covers added and deleted lines.

A

certifi/core.py

Fixes certifi#192.

adamchainz · 2022-06-11T11:33:09Z

Squashed everything together, ready for merge.

tacaswell · 2022-06-22T14:54:16Z

Will this also fix #131 ?

I think this will also actually fix #170, #183, #184, and #190 which were closed without being fixed.

Thank you for this work @adamchainz ! Not sure why 5th time is the charm, but I am grateful #192 was not also closed as a duplicate and your PR is being considered.

Can the simpler #147 be reconsidered (I think this is what @AA-Turner was suggesting?)

alex · 2022-06-22T14:57:34Z

Unfortunately python 3.6 still represents 15% of our downloads, so I think I speak for all the maintainers when I say we're going to be hesitant about the tradeoffs of fixing deprecation warnings by dropping support for such a large group of users.

AA-Turner · 2022-06-22T15:06:24Z

Would the PR be accepted if it incorporated the filesystem branch?

A

alex · 2022-06-22T15:07:51Z

Meaning it maintained support for Python 3.6? Yes.

AA-Turner

Suggested changes restore 3.6 compat. (untested)

A

AA-Turner · 2022-06-22T18:18:39Z

.github/workflows/ci.yml

-        python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"]
-
+        python-version:
+        - "3.7"


Suggested change

- "3.7"

- "3.6"

- "3.7"

AA-Turner · 2022-06-22T18:20:45Z

setup.py

@@ -46,19 +46,20 @@
    include_package_data=True,
    zip_safe=False,
    license='MPL-2.0',
-    python_requires=">=3.6",
+    python_requires=">=3.7",


Suggested change

python_requires=">=3.7",

python_requires=">=3.6",

AA-Turner · 2022-06-22T18:21:15Z

setup.py

    classifiers=[
        'Development Status :: 5 - Production/Stable',
        'Intended Audience :: Developers',
        'License :: OSI Approved :: Mozilla Public License 2.0 (MPL 2.0)',
        'Natural Language :: English',
        'Programming Language :: Python',
        'Programming Language :: Python :: 3',
-        'Programming Language :: Python :: 3.6',
+        'Programming Language :: Python :: 3 :: Only',


Suggested change

'Programming Language :: Python :: 3 :: Only',

'Programming Language :: Python :: 3 :: Only',

'Programming Language :: Python :: 3.6',

AA-Turner · 2022-06-22T18:22:05Z

certifi/core.py

+    def contents() -> str:
+        return files("certifi").joinpath("cacert.pem").read_text(encoding="ascii")
+
+else:


Suggested change

else:

elif sys.version_info >= (3, 7):

AA-Turner · 2022-06-22T18:26:01Z

certifi/core.py

-def contents() -> str:
-    return read_text("certifi", "cacert.pem", encoding="ascii")
+    def contents() -> str:
+        return read_text("certifi", "cacert.pem", encoding="ascii")


Suggested change

return read_text("certifi", "cacert.pem", encoding="ascii")

return read_text("certifi", "cacert.pem", encoding="ascii")

else:

import os

import types

from typing import Union

Package = Union[types.ModuleType, str]

Resource = Union[str, "os.PathLike"]

# This fallback will work for Python versions prior to 3.7 that lack the

# importlib.resources module but relies on the existing `where` function

# so won't address issues with environments like PyOxidizer that don't set

# __file__ on modules.

def read_text(

package: Package,

resource: Resource,

encoding: str = 'utf-8',

errors: str = 'strict'

) -> str:

with open(where(), encoding=encoding) as data:

return data.read()

# If we don't have importlib.resources, then we will just do the old logic

# of assuming we're on the filesystem and munge the path directly.

def where() -> str:

f = os.path.dirname(__file__)

return os.path.join(f, "cacert.pem")

ssbarnea · 2022-07-15T12:50:10Z

@Lukasa Can we please merge this? This seems to be a blocker for enabling us to test against py311 as we treat warnings as errors.

alex · 2022-07-15T12:52:40Z

This PR breaks Python 3.6 support as-is. We have said several times that it will not be considered until 3.6 support is restored.

ssbarnea · 2022-07-15T12:58:46Z

@alex py36 reached end-of-life more than 6 months ago, why not just dropping it? If someone wants to maintain it, it can be done in a maintenance branch.

IMHO, we should to hinder main (brand) development on a dead snake, especially as it can be quite tricky to keep compatibility across a wide range of python versions.

The way I see it is that original PR is more than two months old and soon we might end-up not having a certify for python 3.11 if we don't fix it. What is worse? Still, if agreed, removal of support for py36 should be done in a separated change.

alex · 2022-07-15T13:00:20Z

Python 3.6 reflects a greater percent of our downloads than either Python 3.9 or 3.10.

hugovk · 2022-07-15T13:27:28Z

Please see PR #199, which is the same as this with @AA-Turner's suggestions applied to retain 3.6 support.

hugovk · 2022-07-15T14:23:53Z

Here's the download share for different versions, 3.6 is the third biggest chunk:

https://hugovk.github.io/pypi-tools/charts.html#certifi

ssbarnea · 2022-07-16T14:20:19Z

Thanks for addressing it. Regarding downloads stats, please be aware that they might be painting a picture very different than what people writing code might attempt to use.

if you do not have wheels for a version, it will not be listed, even if people might attempt to use it
if a specific version is buggy or even broken it will likely not get so many downloads
pypi traffic comes from >95% CI/CD implementations for most libraries (exception those "end-user" packages that are not really re-used). I ended up being listed with 16 critical projects, even if some of these were even projects with 1-2 contributors and less than 10 stars on github, basically being spin-offs from our main projects. The only reason for that is because they got a huge number of downloads from our CI/CD pipelines integration (they are also testing tools, so others reuse them for testing).
what the graph is telling you is who is making traffic, many of the old versions are from CI/CD pipelines still using them, often even scheduled ones running on dead projects (yep, seen that -- at least github introduced an option to disable actions after 6mo of inactivity)

That is why I suggested moving an old version to a maintenance branch, so if someone really needs a fix, they would have to backport it, likely having to make other changes to do so, maybe even sponsor/pay maintainers to get it done. I am a maintainer myself and I consider myself (morally) responsable to help others that contribute back. When it comes to support "old" stuff, that is where you can be picky. I seen some very small projects keeping compatibility with only 2 versions of python but that is extreme. Obviously that "certify" is an AAA class project, at least as popularity, not budget, so doing that would be insane.

Again, a bit thanks to all involved!

alex · 2022-07-16T14:27:55Z

It is incorrect that not having wheels impacts download stats. PyPI download stats are computed using the Python version doing the install, from its user agent, not based on the Python version a wheel targets. Indeed, certifi only publishes universal wheels.

Further, neither Python 3.9 nor 3.10 are buggy, so we wouldn't expect that to depress download counts.

While its true that many downloads come from CI systems, these are nonetheless systems that would be impacted by dropping a Python version.

Finally, a maintenance branch makes little sense for this project, given that our primary purpose is to distribute a PEM file. It's not even clear to me how we'd version such a branch.

ssbarnea · 2022-07-16T14:37:56Z

I totally agree with you. Probably this project is the best use-case against using a maintenance branch, as it basically packages a single data file, nothing else.

ssbarnea · 2022-07-18T10:00:06Z

Any chance of doing a patch release so we can start using with py311? I would prefer to not have to add an ignore for the fixed runtime warning.

adamchainz force-pushed the issue_192 branch 3 times, most recently from 298b80a to 33d24fe Compare May 11, 2022 09:44

adamchainz force-pushed the issue_192 branch from 33d24fe to d5613b1 Compare May 19, 2022 17:08

adamchainz force-pushed the issue_192 branch from d5613b1 to bb2297f Compare May 19, 2022 17:16

AA-Turner approved these changes Jun 8, 2022

View reviewed changes

certifi/core.py Outdated Show resolved Hide resolved

certifi/core.py Outdated Show resolved Hide resolved

RhinosF1 mentioned this pull request Jun 8, 2022

mw-sre: support py 3.11 miraheze/puppet#2662

Closed

adamchainz force-pushed the issue_192 branch from 1facbc9 to dfd5739 Compare June 11, 2022 09:27

AA-Turner reviewed Jun 11, 2022

View reviewed changes

setup.py Show resolved Hide resolved

AA-Turner reviewed Jun 11, 2022

View reviewed changes

certifi/core.py Show resolved Hide resolved

certifi/core.py Show resolved Hide resolved

certifi/core.py Outdated Show resolved Hide resolved

Fix deprecation warning on Python 3.11

53e70ad

Fixes certifi#192.

adamchainz force-pushed the issue_192 branch from 797475a to 53e70ad Compare June 11, 2022 11:32

AA-Turner approved these changes Jun 11, 2022

View reviewed changes

coletdjnz mentioned this pull request Jun 13, 2022

[rh:requests] Add support for requests yt-dlp/yt-dlp#3668

Merged

9 tasks

AA-Turner reviewed Jun 22, 2022

View reviewed changes

hugovk mentioned this pull request Jun 26, 2022

Fix/silence doctest warnings python/cpython#94001

Closed

rgommers mentioned this pull request Jul 6, 2022

ENH: Add scipy.datasets submodule scipy/scipy#15607

Merged

6 tasks

ssbarnea approved these changes Jul 15, 2022

View reviewed changes

ssbarnea mentioned this pull request Jul 15, 2022

py311 enablement on ansible devtools projects ansible/team-devtools#58

Closed

13 tasks

hugovk mentioned this pull request Jul 15, 2022

Fix deprecation warning on Python 3.11 #199

Merged

alex closed this in #199 Jul 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix deprecation warning on Python 3.11 #193

Fix deprecation warning on Python 3.11 #193

adamchainz commented May 11, 2022

adamchainz commented May 11, 2022 •

edited

adamchainz commented May 11, 2022

hugovk commented May 19, 2022

adamchainz commented May 19, 2022

AA-Turner left a comment

sigmavirus24 commented Jun 8, 2022

AA-Turner commented Jun 8, 2022

sigmavirus24 commented Jun 9, 2022

adamchainz commented Jun 11, 2022

AA-Turner left a comment

adamchainz commented Jun 11, 2022

tacaswell commented Jun 22, 2022

alex commented Jun 22, 2022

AA-Turner commented Jun 22, 2022

alex commented Jun 22, 2022

AA-Turner left a comment

AA-Turner Jun 22, 2022

AA-Turner Jun 22, 2022

AA-Turner Jun 22, 2022

AA-Turner Jun 22, 2022

AA-Turner Jun 22, 2022 •

edited

ssbarnea commented Jul 15, 2022 •

edited

alex commented Jul 15, 2022

ssbarnea commented Jul 15, 2022

alex commented Jul 15, 2022

hugovk commented Jul 15, 2022

hugovk commented Jul 15, 2022

ssbarnea commented Jul 16, 2022

alex commented Jul 16, 2022

ssbarnea commented Jul 16, 2022

ssbarnea commented Jul 18, 2022

	'Programming Language :: Python :: 3 :: Only',
	'Programming Language :: Python :: 3 :: Only',
	'Programming Language :: Python :: 3.6',

-        return read_text("certifi", "cacert.pem", encoding="ascii")
+        return read_text("certifi", "cacert.pem", encoding="ascii")
+else:
+    import os
+    import types
+    from typing import Union
+    Package = Union[types.ModuleType, str]
+    Resource = Union[str, "os.PathLike"]
+    # This fallback will work for Python versions prior to 3.7 that lack the
+    # importlib.resources module but relies on the existing `where` function
+    # so won't address issues with environments like PyOxidizer that don't set
+    # __file__ on modules.
+    def read_text(
+        package: Package,
+        resource: Resource,
+        encoding: str = 'utf-8',
+        errors: str = 'strict'
+    ) -> str:
+        with open(where(), encoding=encoding) as data:
+            return data.read()
+    # If we don't have importlib.resources, then we will just do the old logic
+    # of assuming we're on the filesystem and munge the path directly.
+    def where() -> str:
+        f = os.path.dirname(__file__)
+        return os.path.join(f, "cacert.pem")

Fix deprecation warning on Python 3.11 #193

Fix deprecation warning on Python 3.11 #193

Conversation

adamchainz commented May 11, 2022

adamchainz commented May 11, 2022 • edited

adamchainz commented May 11, 2022

hugovk commented May 19, 2022

adamchainz commented May 19, 2022

AA-Turner left a comment

Choose a reason for hiding this comment

sigmavirus24 commented Jun 8, 2022

AA-Turner commented Jun 8, 2022

sigmavirus24 commented Jun 9, 2022

adamchainz commented Jun 11, 2022

AA-Turner left a comment

Choose a reason for hiding this comment

adamchainz commented Jun 11, 2022

tacaswell commented Jun 22, 2022

alex commented Jun 22, 2022

AA-Turner commented Jun 22, 2022

alex commented Jun 22, 2022

AA-Turner left a comment

Choose a reason for hiding this comment

AA-Turner Jun 22, 2022

Choose a reason for hiding this comment

AA-Turner Jun 22, 2022

Choose a reason for hiding this comment

AA-Turner Jun 22, 2022

Choose a reason for hiding this comment

AA-Turner Jun 22, 2022

Choose a reason for hiding this comment

AA-Turner Jun 22, 2022 • edited

Choose a reason for hiding this comment

ssbarnea commented Jul 15, 2022 • edited

alex commented Jul 15, 2022

ssbarnea commented Jul 15, 2022

alex commented Jul 15, 2022

hugovk commented Jul 15, 2022

hugovk commented Jul 15, 2022

ssbarnea commented Jul 16, 2022

alex commented Jul 16, 2022

ssbarnea commented Jul 16, 2022

ssbarnea commented Jul 18, 2022

adamchainz commented May 11, 2022 •

edited

AA-Turner Jun 22, 2022 •

edited

ssbarnea commented Jul 15, 2022 •

edited