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

Test python 3.12 #175

Merged
merged 4 commits into from
Aug 23, 2024
Merged

Conversation

white-gecko
Copy link
Contributor

@white-gecko white-gecko commented Aug 19, 2024

Requires: #172

Add python 3.12 to the build matrix.
Install setuptools in test environment for python 3.12 (cf. #168).

@wumpus
Copy link
Collaborator

wumpus commented Aug 21, 2024

I ran with 3.12 prior to the change in urllib3 version and it worked. The pin to that particular old urllib3 version was related to proxy http/https, as @tw4l mentioned a few days ago. This error ModuleNotFoundError: No module named 'urllib3.packages.six.moves' doesn't seem to have an obvious cause, when I google it.

@wumpus
Copy link
Collaborator

wumpus commented Aug 21, 2024

This CI failure was caused by pypi timing out. I'll try it again in a while.

@wumpus
Copy link
Collaborator

wumpus commented Aug 21, 2024

Hm. I made my own branch that tests 3.12 and uses the exact urllib3 version and no bueno.

@white-gecko
Copy link
Contributor Author

Locally on my machine it works with python 3.12, I also made an integration branch which succeeded on github: https://github.com/white-gecko/warcio/actions/runs/10452831491/job/28944239907

@white-gecko
Copy link
Contributor Author

I have rebased the branch on the current master, according to my experiments that should work.

@white-gecko
Copy link
Contributor Author

The urllib3==1.25.11 dependency causes ModuleNotFoundError: No module named 'urllib3.packages.six.moves' on python 3.12. At least 1.26.5 is required. It can also be seen in the urllib3 changelog, that an update to the six dependency happened: https://github.com/urllib3/urllib3/blob/main/CHANGES.rst#1265-2021-05-26.
I have just adjusted the minimum version from 1.26.4 to 1.26.5.

@wumpus
Copy link
Collaborator

wumpus commented Aug 22, 2024

OK, I think this is fully safe. Thanks @white-gecko for working out this fix and also for figuring out that at some point urllib3 fixed the http/https proxy thing that motivated us pinning the version in the first place.

I still have 2 questions:

  1. why the upper bound in 'urllib3>=1.26.5,<1.26.16',?
  2. can we future-proof installing setuptools for python 3.12 and up? Right now it's only exactly 3.12.

@wumpus wumpus merged commit 2ff571f into webrecorder:master Aug 23, 2024
6 checks passed
@wumpus
Copy link
Collaborator

wumpus commented Aug 23, 2024

I can answer both of my questions myself, I suppose.

@white-gecko
Copy link
Contributor Author

OK, I think this is fully safe. Thanks @white-gecko for working out this fix and also for figuring out that at some point urllib3 fixed the http/https proxy thing that motivated us pinning the version in the first place.

I still have 2 questions:

1. why the upper bound in `'urllib3>=1.26.5,<1.26.16',`?

2. can we future-proof installing `setuptools` for python 3.12 and up? Right now it's only exactly 3.12.

Thank you for raising the question. It might be relevant to document the decisions.

  1. The upper bound is required, because in versions 1.26.16 and above the https proxy test runs indefinitely. So once this is fixed, the upper bound can be removed.
  2. Regarding the version command, setuptools is not required anymore. To run setup.py it is still required. Can you provide more detail wht are the issues of future-proof installing setuptools? Maybe an extra issue would help to keep track of this.

@white-gecko white-gecko deleted the feature/python312Support branch August 23, 2024 07:54
@wumpus
Copy link
Collaborator

wumpus commented Aug 23, 2024

For future-proofing, you were installing setuptools only if the python version was exactly 3.12 ... I made it a >= while trying to get 3.13-rc to work, and that worked fine, although 3.13-rc itself failed because the greenlet wheel doesn't build. (This is typical when trying out release candidates...)

      - name: Install setuptools on python 3.12
        if: ${{ matrix.python-version == '3.12' }}
        run: |
          pip install setuptools

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