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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG+1] Drop py34 support - Update CI envs #3892

Merged
merged 17 commits into from Aug 7, 2019
Merged

Conversation

@elacuesta
Copy link
Member

@elacuesta elacuesta commented Jul 21, 2019

Based on #3825 (comment) by @dangra

I personally would like to see this because type hints were introduced in py35, and I'm currently working on #3881 (although it seems like variable annotations were introduced in 3.6 馃樋 - PEP 526)

@codecov
Copy link

@codecov codecov bot commented Jul 21, 2019

Codecov Report

Merging #3892 into master will decrease coverage by 3.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3892      +/-   ##
==========================================
- Coverage   85.56%   82.55%   -3.02%     
==========================================
  Files         164      164              
  Lines        9565     9565              
  Branches     1435     1435              
==========================================
- Hits         8184     7896     -288     
- Misses       1133     1409     +276     
- Partials      248      260      +12
Impacted Files Coverage 螖
scrapy/linkextractors/sgml.py 0% <0%> (-96.81%) 猬囷笍
scrapy/linkextractors/regex.py 0% <0%> (-95.66%) 猬囷笍
scrapy/linkextractors/htmlparser.py 0% <0%> (-92.07%) 猬囷笍
scrapy/core/downloader/handlers/s3.py 62.9% <0%> (-32.26%) 猬囷笍
scrapy/extensions/statsmailer.py 0% <0%> (-30.44%) 猬囷笍
scrapy/utils/boto.py 46.66% <0%> (-26.67%) 猬囷笍
scrapy/_monkeypatches.py 42.85% <0%> (-14.29%) 猬囷笍
scrapy/link.py 86.36% <0%> (-13.64%) 猬囷笍
scrapy/core/downloader/tls.py 77.5% <0%> (-12.5%) 猬囷笍
scrapy/utils/gz.py 91.89% <0%> (-8.11%) 猬囷笍
... and 17 more

@codecov
Copy link

@codecov codecov bot commented Jul 21, 2019

Codecov Report

Merging #3892 into master will decrease coverage by 0.58%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3892      +/-   ##
==========================================
- Coverage   85.54%   84.96%   -0.59%     
==========================================
  Files         166      166              
  Lines        9681     9681              
  Branches     1445     1445              
==========================================
- Hits         8282     8225      -57     
- Misses       1146     1194      +48     
- Partials      253      262       +9
Impacted Files Coverage 螖
scrapy/core/downloader/handlers/s3.py 62.9% <0%> (-32.26%) 猬囷笍
scrapy/utils/boto.py 46.66% <0%> (-26.67%) 猬囷笍
scrapy/core/downloader/tls.py 75.92% <0%> (-12.97%) 猬囷笍
scrapy/utils/ssl.py 47.22% <0%> (-5.56%) 猬囷笍
scrapy/extensions/feedexport.py 78.44% <0%> (-5.05%) 猬囷笍
scrapy/core/downloader/handlers/http11.py 89.92% <0%> (-2.62%) 猬囷笍
scrapy/core/scraper.py 86.48% <0%> (-2.03%) 猬囷笍
scrapy/pipelines/files.py 65.38% <0%> (-1.16%) 猬囷笍

setup.py Outdated Show resolved Hide resolved
@elacuesta elacuesta force-pushed the drop_py34 branch 4 times, most recently from 34321e6 to d9e8bd6 Jul 21, 2019
@elacuesta
Copy link
Member Author

@elacuesta elacuesta commented Jul 21, 2019

The py2 tests are failing due to a problem in a recent docutils upgrade: https://sourceforge.net/p/docutils/bugs/365/.
I don't think it's worth pinning the docutils dependency unless they don't fix it upstream quickly, which I think it should probably happen given the amount of builds that are probably failing because of that.
I deleted my previous comments about it to avoid cluttering the PR.

@elacuesta elacuesta closed this Jul 22, 2019
@elacuesta elacuesta reopened this Jul 22, 2019
@kmike
Copy link
Member

@kmike kmike commented Jul 23, 2019

hey @elacuesta! I think it makes sense to do this as a part of dropping support for old OS, setting a new baseline Ubuntu/Debian. We need to decide what's the baseline, and then bump minimum package requirements and minimum Python version based on that.

@kmike
Copy link
Member

@kmike kmike commented Jul 23, 2019

My proposal:

Ubuntu 14.04 trusty in only supported for paid customers (if I read announcements correctly), and it uses Python 3.4, twisted 13.2.0 (no Python 3 support), lxml 3.3.3.

Debian jessie has LTS support until mid-2020. But it is also getting old (Python 3.4.2, Twisted 14.0.2 without Python 3 support, lxml 3.4).

Is it too aggressive? //cc @dangra

@elacuesta
Copy link
Member Author

@elacuesta elacuesta commented Jul 23, 2019

Xenial sounds good to me 馃憤
According to the Travis docs Trusty is the default distribution, and it's the one we're currently using for all builds except for the Python 3.7 one.
Excuse my ignorance, but I'm failing to see how the Debian tests are executed. There is no Debian option in the docs I mentioned, and in fact the builds with TOXENV=jessie have Build dist: trusty under the Build system information section at the beginning of the logs.

@elacuesta elacuesta force-pushed the drop_py34 branch 2 times, most recently from edd2f13 to 87c0976 Jul 23, 2019
@wRAR
Copy link
Contributor

@wRAR wRAR commented Jul 24, 2019

@elacuesta jessie is emulated by installing the specific versions of modules into a trusty system.

@wRAR
Copy link
Contributor

@wRAR wRAR commented Jul 24, 2019

There is a CI on https://salsa.debian.org, I planned to learn it and maybe set up something automatical one day.

@elacuesta
Copy link
Member Author

@elacuesta elacuesta commented Jul 24, 2019

@elacuesta jessie is emulated by installing the specific versions of modules into a trusty system.

Oh nice, that's what I suspected but I wasn't sure. Thanks!

@kmike
Copy link
Member

@kmike kmike commented Jul 25, 2019

@elacuesta: we agreed to use Ubuntu 16.04 and Debian stretch as a new baseline for the 1.8 release. Feel free to update tox environments, requirements, Python versions, etc. to match it :)

@kmike kmike added this to the v1.8 milestone Jul 26, 2019
@elacuesta elacuesta force-pushed the drop_py34 branch 3 times, most recently from ead680d to d9543a6 Jul 26, 2019
@elacuesta
Copy link
Member Author

@elacuesta elacuesta commented Aug 2, 2019

Hello everyone. I updated this PR to add an environment with pinned versions that are known to work well. I'm only running it on py36, I think that's probably the most reasonable compromise to avoid adding many more envs to the Travis matrix, specially now that we have two more for the Robots parsing. I updated the requirements-py3.txt file with a few comments explaining the reasons for the picked versions.
To be honest I didn't pay much attention to the py2 tests, I'm not sure it's worth at this point.
Let me know what you think, and as usual, thanks for your comments 馃殌

@elacuesta elacuesta changed the title Drop py34 support Drop py34 support - Update CI envs Aug 2, 2019
@kmike
Copy link
Member

@kmike kmike commented Aug 2, 2019

Thanks @elacuesta!

I'm only running it on py36

Why not 3.5? Oldest Python, oldest supported versions of dependencies?

To be honest I didn't pay much attention to the py2 tests, I'm not sure it's worth at this point.

I think it worths it, as we may end up supporting 1.8 a bit longer than other versions. There is also a different Twisted minimum version requirement in setup.py; if we're not bumping it to match Python 3, it would be good to have a py27-pinned-versions env as well.

setup.py Outdated Show resolved Hide resolved
@elacuesta
Copy link
Member Author

@elacuesta elacuesta commented Aug 2, 2019

Hey @kmike, I made some updates based on your feedback. Thanks!

Copy link
Member

@Gallaecio Gallaecio left a comment

Looks good. I鈥檝e left a few minor comments regarding pinned versions.

tox.ini Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@elacuesta
Copy link
Member Author

@elacuesta elacuesta commented Aug 6, 2019

Updated, added minimum and pinned versions of cryptography and zope.interface. botocore and Pillow are only installed for the tests, as they are only required by certain parts (S3 feed storage and images pipeline IIRC) and are not required for Scrapy itself to run.

@elacuesta
Copy link
Member Author

@elacuesta elacuesta commented Aug 6, 2019

Random Travis error about not being able to install tox in one of the py27 envs. All other tests are green, closing and reopening to trigger another build.

@elacuesta elacuesta closed this Aug 6, 2019
@elacuesta elacuesta reopened this Aug 6, 2019
@Gallaecio Gallaecio changed the title Drop py34 support - Update CI envs [MRG+1] Drop py34 support - Update CI envs Aug 7, 2019
@kmike kmike merged commit 5dbeece into scrapy:master Aug 7, 2019
2 checks passed
@kmike
Copy link
Member

@kmike kmike commented Aug 7, 2019

Thanks @elacuesta! Farewell Python 3.4!

@elacuesta elacuesta deleted the drop_py34 branch Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants