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

Replace imp with importlib #365

Merged
merged 2 commits into from
Sep 27, 2023
Merged

Replace imp with importlib #365

merged 2 commits into from
Sep 27, 2023

Conversation

gump
Copy link
Contributor

@gump gump commented Sep 8, 2023

This project uses the imp module which has been deprecated since Python 3.4 and set for removal in 3.12:

  • Raised PendingDeprecationWarning since 3.4 (2014)
  • Raised DeprecationWarning since 3.5 (2015)
  • Updated DeprecationWarning to say removal in 3.12 since 3.10 (2021)
  • Removal planned for 3.12 (2023)

This change removes the dependency on imp in favour of importlib.

Co-authored-by: @jbkkd
Inspired by: @mgorny

#358

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #365 (056fe0b) into master (dce5f37) will decrease coverage by 0.05%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##           master     #365      +/-   ##
==========================================
- Coverage   89.98%   89.93%   -0.05%     
==========================================
  Files          25       25              
  Lines        1198     1202       +4     
  Branches      216      217       +1     
==========================================
+ Hits         1078     1081       +3     
  Misses         89       89              
- Partials       31       32       +1     
Files Coverage Δ
configurations/importer.py 84.74% <90.90%> (-0.35%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

This project uses the imp module which has been deprecated since Python 3.4 and set for removal in 3.12:
• Raised PendingDeprecationWarning since 3.4 (2014)
• Raised DeprecationWarning since 3.5 (2015)
• Updated DeprecationWarning to say removal in 3.12 since 3.10 (2021)
• Removal planned for 3.12 (2023)

This change removes the dependency on imp in favour of importlib.

Co-authored-by: @jbkkd
Inspired by: @mgorny

jazzband#358
@gump
Copy link
Contributor Author

gump commented Sep 20, 2023

Seems that the pypy builds are failing due to typing issues that are not related to changes in this PR at all. I'm not sure how to best tackle this. If anyone has ideas on how to solve this please shout!

  pypy38-dj42 installdeps: django~=4.2.0, coverage, coverage_enable_subprocess
  [2171] /home/runner/work/django-configurations/django-configurations$ /home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/bin/python -m pip install 'django~=4.2.0' coverage coverage_enable_subprocess >.tox/pypy38-dj42/log/pypy38-dj42-1.log
  pypy38-dj42 develop-inst: /home/runner/work/django-configurations/django-configurations
  write config to /home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/.tox-config1 as '5f83b1adfc4e93325d54b10bf36fb935a03a746487f791180f644a357ed6f6ab /opt/hostedtoolcache/PyPy/3.8.16/x64/bin/python\n3.28.0 0 1 0\n00000000000000000000000000000000 django~=4.2.0\n00000000000000000000000000000000 coverage\n00000000000000000000000000000000 coverage_enable_subprocess'
  [2181] /home/runner/work/django-configurations/django-configurations$ /home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/bin/python -m pip install --exists-action w -e '/home/runner/work/django-configurations/django-configurations[testing]' >.tox/pypy38-dj42/log/pypy38-dj42-2.log
  [2222] /home/runner/work/django-configurations/django-configurations$ /home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/bin/python -m pip freeze >.tox/pypy38-dj42/log/pypy38-dj42-3.log
  pypy38-dj42 installed: asgiref==3.7.2,backports.zoneinfo==0.2.1,cffi==1.15.1,coverage==7.3.1,coverage-enable-subprocess==1.0,dj-database-url==2.1.0,dj-email-url==1.0.6,dj-search-url==0.1,Django==4.2.5,django-cache-url==3.4.4,-e git+https://github.com/jazzband/django-configurations@bc02e9dd35fcecaf462847caf7dd56f46bcf40ec#egg=django_configurations,greenlet==0.4.13,hpy==0.0.4.dev[179](https://github.com/jazzband/django-configurations/actions/runs/6123560281/job/16621808225?pr=365#step:7:183)+g9b5d200,readline==6.2.4.1,sqlparse==0.4.4,typing_extensions==4.7.1
  pypy38-dj42 run-test-pre: PYTHONHASHSEED='546480419'
  pypy38-dj42 run-test: commands[0] | python --version
  [2228] /home/runner/work/django-configurations/django-configurations$ /home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/bin/python --version
  Python 3.8.16 (a9dbdca6fc3286b0addd2240f11d97d8e8de[187](https://github.com/jazzband/django-configurations/actions/runs/6123560281/job/16621808225?pr=365#step:7:191)a, Dec 29 2022, 11:45:13)
  [PyPy 7.3.11 with GCC 10.2.1 20210130 (Red Hat 10.2.1-11)]
  pypy38-dj42 run-test: commands[1] | /home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/bin/coverage run /home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/bin/django-cadmin test -v2 tests
  [2229] /home/runner/work/django-configurations/django-configurations$ /home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/bin/coverage run .tox/pypy38-dj42/bin/django-cadmin test -v2 tests
  Traceback (most recent call last):
    File ".tox/pypy38-dj42/bin/django-cadmin", line 33, in <module>
      sys.exit(load_entry_point('django-configurations', 'console_scripts', 'django-cadmin')())
    File ".tox/pypy38-dj42/bin/django-cadmin", line 25, in importlib_load_entry_point
      return next(matches).load()
    File "/opt/hostedtoolcache/PyPy/3.8.16/x64/lib/pypy3.8/importlib/metadata.py", line 77, in load
      module = import_module(match.group('module'))
    File "/opt/hostedtoolcache/PyPy/3.8.16/x64/lib/pypy3.8/importlib/__init__.py", line 127, in import_module
      return _bootstrap._gcd_import(name[level:], package, level)
    File "<frozen importlib._bootstrap>", line 1023, in _gcd_import
    File "<frozen importlib._bootstrap>", line 1000, in _find_and_load
    File "<frozen importlib._bootstrap>", line 970, in _find_and_load_unlocked
    File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
    File "<frozen importlib._bootstrap>", line 1023, in _gcd_import
    File "<frozen importlib._bootstrap>", line 1000, in _find_and_load
    File "<frozen importlib._bootstrap>", line 984, in _find_and_load_unlocked
    File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
    File "<builtin>/frozen importlib._bootstrap_external", line 858, in exec_module
    File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
    File "/home/runner/work/django-configurations/django-configurations/configurations/__init__.py", line 1, in <module>
      from .base import Configuration  # noqa
    File "/home/runner/work/django-configurations/django-configurations/configurations/base.py", line 4, in <module>
      from django.conf import global_settings
    File "/home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/lib/pypy3.8/site-packages/django/conf/__init__.py", line 19, in <module>
      from django.utils.deprecation import RemovedInDjango50Warning, RemovedInDjango51Warning
    File "/home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/lib/pypy3.8/site-packages/django/utils/deprecation.py", line 4, in <module>
      from asgiref.sync import iscoroutinefunction, markcoroutinefunction, sync_to_async
    File "/home/runner/work/django-configurations/django-configurations/.tox/pypy38-dj42/lib/pypy3.8/site-packages/asgiref/sync.py", line 130, in <module>
      class AsyncToSync(Generic[_P, _R]):
    File "/opt/hostedtoolcache/PyPy/3.8.16/x64/lib/pypy3.8/typing.py", line 261, in inner
      return func(*args, **kwds)
    File "/opt/hostedtoolcache/PyPy/3.8.16/x64/lib/pypy3.8/typing.py", line 890, in __class_getitem__
      raise TypeError(
TypeError: Parameters to Generic[...] must all be type variables

@washeck
Copy link
Contributor

washeck commented Sep 22, 2023

@gump I'm not sure if this should block the PR because there is exactly the same error on master (tested on pypy39-dj42 with tox on my machine).

@washeck
Copy link
Contributor

washeck commented Sep 22, 2023

I believe the root cause is that asgiref is using typing_extension on python 3.9 and lower (including pypy) and we are running into python/typing_extensions#126

I verified that the issue does not exist on pypy3.10 (which does not use typing_extensions). My suggestion would be to merge this PR and drop support for PyPy < 3.10.

@gump
Copy link
Contributor Author

gump commented Sep 25, 2023

Thanks @washeck. I can see you already prepared #366 which adds pypy3.10 to the testing pipeline.

I don't have the powers to merge this so will need to wait for someone with the powers to do the honours.

@pauloxnet
Copy link
Member

@washeck @gump I opened PR #369 to upgrade Python/Django dependencies and fix the PyPy-related issue to permit a new package release.
Can you please review it?

@washeck
Copy link
Contributor

washeck commented Oct 4, 2023

@pauloxnet LGTM. I didn't do it in my PR as I didn't know what the maintainers want. Looking forward to merge of your PR and release of 2.5.

@pauloxnet
Copy link
Member

@pauloxnet LGTM.

Can you please use the "Review changes" button to approve my PR
https://github.com/jazzband/django-configurations/pull/369/files

@washeck
Copy link
Contributor

washeck commented Oct 4, 2023

Can you please use the "Review changes" button to approve my PR https://github.com/jazzband/django-configurations/pull/369/files

Sorry, I didn't know I have the permission. Approved now.

@pauloxnet
Copy link
Member

Can you please use the "Review changes" button to approve my PR https://github.com/jazzband/django-configurations/pull/369/files

Sorry, I didn't know I have the permission. Approved now.

Thanks 👍

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.

4 participants