jazzband / django-hosts

Dynamic and static host resolving for Django. Maps hostnames to URLconfs.
http://django-hosts.rtfd.org
Other
982 stars 106 forks source link

Support Python 3.12 and drop 3.7 #164

Closed max-muoto closed 12 months ago

max-muoto commented 12 months ago

As mentioned in this issue https://github.com/jazzband/django-hosts/issues/163 pkg_resources is deprecated and importlib should be used instead. Putting out this PR as this is blocking our upgrade to Python 3.12

codecov[bot] commented 12 months ago

Codecov Report

Merging #164 (ada93da) into master (4d4ef69) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #164   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          400       400           
  Branches        68        68           
=========================================
  Hits           400       400           
Files Coverage Ξ”
django_hosts/__init__.py 100.00% <100.00%> (ΓΈ)

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

max-muoto commented 12 months ago

Before merging this in, one issue is that 3.7 users might not have the importlib_metadata backport installed. Seeing this library doesn't have any external requirements. Are we fine with adding the backport in as a requirement for 3.7 users, or is there any thought around dropping support for 3.7?

ddabble commented 12 months ago

Good point! I'm personally planning on opening a PR removing support for Python 3.7, since its EOL has passed (as has been done with previous Python versions), but presuming this PR will be merged before that, we should probably keep the code using pkg_resources for Python versions < 3.7..

Any thoughts? Would you mind making that change? πŸ™‚

max-muoto commented 12 months ago

Good point! I'm personally planning on opening a PR removing support for Python 3.7, since its EOL has passed (as has been done with previous Python versions), but presuming this PR will be merged before that, we should probably keep the code using pkg_resources for Python versions < 3.7..

Any thoughts? Would you mind making that change? πŸ™‚

Sure, let me go ahead and package that in with this PR! I'll also add a CI check for 3.12.

ddabble commented 12 months ago

I'll also add a CI check for 3.12.

Sure, given that it passes πŸ˜…

max-muoto commented 12 months ago

I'll also add a CI check for 3.12.

Sure, given that it passes πŸ˜…

Went ahead and removed the conditional import since we can now expect that all users will have importlib.metadata and added CI checks if you want to run the pipeline.

max-muoto commented 12 months ago

@ddabble Looks like all checks are passing, and for 3.12, any blockers from merging this?

max-muoto commented 12 months ago

Hm, it's now moved a little too far out of scope of the original PR, in my opinion, but if we rename it to e.g. "Support Python 3.12 and drop 3.7", it should be good :)

Good callout, went ahead and renamed it!

I pushed a few commits to add some changes I think were missing. Do you mind if I reorganize the commits before merging? I was thinking one commit for dropping 3.7 and one for supporting 3.12.

Thanks for doing that! Sure thing, I didn't know if they would get squashed or not, so appreciate it!

ddabble commented 12 months ago

Btw, I noticed that you removed testing Django 4.1 with Python 3.12 in 3aea2022b4657cf6be56d984361d08a463040eb7, but it seems like the tests pass after undoing it in e293a761863b4c489cf50aa5e5fbc131662493f7. Any particular reason..? πŸ™‚

max-muoto commented 12 months ago

Btw, I noticed that you removed testing Django 4.1 with Python 3.12 in 3aea202, but it seems like the tests pass after undoing it in e293a76. Any particular reason..? πŸ™‚

When doing that, wasn't 100% sure if it would pass, and saw that official support for 3.12 was only going to be a part of 4.2.

ddabble commented 12 months ago

Ah, I see! I guess we should ideally keep your version, then :)