search

webrecorder / warcio

Streaming WARC/ARC library for fast web archive IO
https://pypi.python.org/pypi/warcio
Apache License 2.0
387 stars 58 forks source link

Test python 3.12 #175

Closed white-gecko closed 3 months ago

white-gecko commented 3 months ago

Requires: #172

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

wumpus commented 3 months ago

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 commented 3 months ago

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

wumpus commented 3 months ago

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

white-gecko commented 3 months ago

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 commented 3 months ago

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

white-gecko commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

white-gecko commented 3 months ago

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.
wumpus commented 3 months ago

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