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

Python 3.10 compatibility #166

Merged
merged 17 commits into from Mar 10, 2022
Merged

Conversation

elacuesta
Copy link
Member

@elacuesta elacuesta commented Mar 9, 2022

Update some imports and test requirements for py310 compatibility.

Small overlap with #159, I think we can merge this sooner.

@elacuesta elacuesta requested review from wRAR and Gallaecio March 9, 2022 15:37
@codecov
Copy link

codecov bot commented Mar 9, 2022

Codecov Report

Merging #166 (5adfb9c) into master (34ecb80) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
- Coverage   94.27%   94.27%   -0.01%     
==========================================
  Files          28       28              
  Lines        1958     1957       -1     
==========================================
- Hits         1846     1845       -1     
  Misses        112      112              
Impacted Files Coverage Δ
scrapinghub/client/frontiers.py 89.25% <ø> (ø)
scrapinghub/client/items.py 100.00% <ø> (ø)
scrapinghub/client/projects.py 94.59% <ø> (ø)
scrapinghub/client/proxy.py 95.71% <ø> (ø)
scrapinghub/client/collections.py 92.30% <100.00%> (ø)
scrapinghub/hubstorage/resourcetype.py 97.39% <100.00%> (-0.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34ecb80...5adfb9c. Read the comment docs.

@elacuesta
Copy link
Member Author

elacuesta commented Mar 9, 2022

For some reason, any version of pytest greater than 3.7.0 breaks the tests with
requests.exceptions.ConnectionError: HTTPConnectionPool(host='storage.vm.scrapinghub.com', port=80): Max retries exceeded with url: /ids/2222222/spider/hs-test-spider?create=True (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7f05c493fbe0>: Failed to establish a new connection: [Errno 113] No route to host')) (see https://github.com/scrapinghub/python-scrapinghub/runs/5484969907)

That is a problem for py310, because of pytest-dev/pytest#8540 (see https://github.com/scrapinghub/python-scrapinghub/runs/5484724225), which is available since 6.2.4

@Gallaecio
Copy link
Member

Gallaecio commented Mar 9, 2022

I see E AttributeError: module 'collections' has no attribute 'Iterable' in the latest test failures, which sounds like #160

@elacuesta
Copy link
Member Author

I see E AttributeError: module 'collections' has no attribute 'Iterable' in the latest test failures, which sounds like #160

That particular error should have been fixed by 1f6f39a, I don't see it in the most recent runs.

@Gallaecio
Copy link
Member

not isinstance(keys, collections.Iterable)):

@elacuesta
Copy link
Member Author

🤦
Thanks, should be fixed by a29a80f. The requests.exceptions.ConnectionError problem seems to be still present though 😓

@Gallaecio
Copy link
Member

Backporting the Python 3.10 fix I have been able to identify pytest-dev/pytest#3754 as the cause of the issue. Things work with the prior upstream commit.

I have pushed at https://github.com/Gallaecio/pytest/tree/3.7.0-3.7.1-fork the latest working pytest with the Python 3.10 fix backported, in case you want to use that in the meantime.

I will continue looking into it for a more permanent fix.

requirements-test.txt Outdated Show resolved Hide resolved
@elacuesta
Copy link
Member Author

I restored py27 and py35 support, it's not strictly necessary to remove them in order to support py310

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.

None yet

2 participants