From 0946eb335a285e1f210ba1185a564699f53b17d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 14 Nov 2019 17:56:21 +0100 Subject: [PATCH 01/22] =?UTF-8?q?Port=20code=20from=20Twisted=E2=80=99s=20?= =?UTF-8?q?deprecated=20HTTPClientFactory=20into=20ScrapyHTTPClientFactory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/core/downloader/webclient.py | 90 ++++++++++++++++++++++------- 1 file changed, 70 insertions(+), 20 deletions(-) diff --git a/scrapy/core/downloader/webclient.py b/scrapy/core/downloader/webclient.py index 3fe13414a..16fd214a3 100644 --- a/scrapy/core/downloader/webclient.py +++ b/scrapy/core/downloader/webclient.py @@ -1,9 +1,9 @@ from time import time from six.moves.urllib.parse import urlparse, urlunparse, urldefrag -from twisted.web.client import HTTPClientFactory from twisted.web.http import HTTPClient -from twisted.internet import defer +from twisted.internet import defer, reactor +from twisted.internet.protocol import ClientFactory from scrapy.http import Headers from scrapy.utils.httpobj import urlparse_cached @@ -93,18 +93,30 @@ class ScrapyHTTPPageGetter(HTTPClient): (self.factory.url, self.factory.timeout))) -class ScrapyHTTPClientFactory(HTTPClientFactory): - """Scrapy implementation of the HTTPClientFactory overwriting the - setUrl method to make use of our Url object that cache the parse - result. - """ +class ScrapyHTTPClientFactory(ClientFactory): protocol = ScrapyHTTPPageGetter + waiting = 1 noisy = False followRedirect = False afterFoundGet = False + def _build_response(self, body, request): + request.meta['download_latency'] = self.headers_time-self.start_time + status = int(self.status) + headers = Headers(self.response_headers) + respcls = responsetypes.from_args(headers=headers, url=self._url) + return respcls(url=self._url, status=status, headers=headers, body=body) + + def _set_connection_attributes(self, request): + parsed = urlparse_cached(request) + self.scheme, self.netloc, self.host, self.port, self.path = _parsed_url_args(parsed) + proxy = request.meta.get('proxy') + if proxy: + self.scheme, _, self.host, self.port, _ = _parse(proxy) + self.path = self.url + def __init__(self, request, timeout=180): self._url = urldefrag(request.url)[0] # converting to bytes to comply to Twisted interface @@ -139,21 +151,59 @@ class ScrapyHTTPClientFactory(HTTPClientFactory): elif self.method == b'POST': self.headers['Content-Length'] = 0 - def _build_response(self, body, request): - request.meta['download_latency'] = self.headers_time-self.start_time - status = int(self.status) - headers = Headers(self.response_headers) - respcls = responsetypes.from_args(headers=headers, url=self._url) - return respcls(url=self._url, status=status, headers=headers, body=body) + def __repr__(self): + return "<%s: %s>" % (self.__class__.__name__, self.url) - def _set_connection_attributes(self, request): - parsed = urlparse_cached(request) - self.scheme, self.netloc, self.host, self.port, self.path = _parsed_url_args(parsed) - proxy = request.meta.get('proxy') - if proxy: - self.scheme, _, self.host, self.port, _ = _parse(proxy) - self.path = self.url + def _cancelTimeout(self, result, timeoutCall): + if timeoutCall.active(): + timeoutCall.cancel() + return result + + def buildProtocol(self, addr): + p = ClientFactory.buildProtocol(self, addr) + p.followRedirect = self.followRedirect + p.afterFoundGet = self.afterFoundGet + if self.timeout: + timeoutCall = reactor.callLater(self.timeout, p.timeout) + self.deferred.addBoth(self._cancelTimeout, timeoutCall) + return p def gotHeaders(self, headers): self.headers_time = time() self.response_headers = headers + + def gotStatus(self, version, status, message): + """ + Set the status of the request on us. + @param version: The HTTP version. + @type version: L{bytes} + @param status: The HTTP status code, an integer represented as a + bytestring. + @type status: L{bytes} + @param message: The HTTP status message. + @type message: L{bytes} + """ + self.version, self.status, self.message = version, status, message + + def page(self, page): + if self.waiting: + self.waiting = 0 + self.deferred.callback(page) + + def noPage(self, reason): + if self.waiting: + self.waiting = 0 + self.deferred.errback(reason) + + def clientConnectionFailed(self, _, reason): + """ + When a connection attempt fails, the request cannot be issued. If no + result has yet been provided to the result Deferred, provide the + connection failure reason as an error result. + """ + if self.waiting: + self.waiting = 0 + # If the connection attempt failed, there is nothing more to + # disconnect, so just fire that Deferred now. + self._disconnectedDeferred.callback(None) + self.deferred.errback(reason) From 42954d0df9a78e6641996fbf2eb76afdef9c1856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 20 Nov 2019 08:16:33 +0100 Subject: [PATCH 02/22] Mention that ScrapyHTTPClientFactory has Twisted code --- scrapy/core/downloader/webclient.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scrapy/core/downloader/webclient.py b/scrapy/core/downloader/webclient.py index 16fd214a3..26726afd4 100644 --- a/scrapy/core/downloader/webclient.py +++ b/scrapy/core/downloader/webclient.py @@ -93,6 +93,10 @@ class ScrapyHTTPPageGetter(HTTPClient): (self.factory.url, self.factory.timeout))) +# This class used to inherit from Twisted’s +# twisted.web.client.HTTPClientFactory. When that class was deprecated in +# Twisted (https://github.com/twisted/twisted/pull/643), we merged its +# non-overriden code into this class. class ScrapyHTTPClientFactory(ClientFactory): protocol = ScrapyHTTPPageGetter From ece4fa6c7cf7fdce9e5e77f56b209941ad401f53 Mon Sep 17 00:00:00 2001 From: nyov Date: Sat, 7 Mar 2020 19:21:45 +0000 Subject: [PATCH 03/22] Fix ignored testcase: boto is never installed --- tests/test_feedexport.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 37384081a..21da9fdcd 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -182,9 +182,9 @@ class S3FeedStorageTest(unittest.TestCase): create=True) def test_parse_credentials(self): try: - import boto # noqa: F401 + import botocore # noqa: F401 except ImportError: - raise unittest.SkipTest("S3FeedStorage requires boto") + raise unittest.SkipTest("S3FeedStorage requires botocore") aws_credentials = {'AWS_ACCESS_KEY_ID': 'settings_key', 'AWS_SECRET_ACCESS_KEY': 'settings_secret'} crawler = get_crawler(settings_dict=aws_credentials) From 234c8b8c50a2db0d6d71455c382957099f4c0dcc Mon Sep 17 00:00:00 2001 From: nyov Date: Sat, 7 Mar 2020 19:31:56 +0000 Subject: [PATCH 04/22] Removing deprecated S3FeedStorage without AWS keys instancing. --- scrapy/extensions/feedexport.py | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 72a34ae0d..e793c12dc 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -94,23 +94,10 @@ class FileFeedStorage: class S3FeedStorage(BlockingFeedStorage): def __init__(self, uri, access_key=None, secret_key=None, acl=None): - # BEGIN Backward compatibility for initialising without keys (and - # without using from_crawler) - no_defaults = access_key is None and secret_key is None - if no_defaults: - from scrapy.utils.project import get_project_settings - settings = get_project_settings() - if 'AWS_ACCESS_KEY_ID' in settings or 'AWS_SECRET_ACCESS_KEY' in settings: - warnings.warn( - "Initialising `scrapy.extensions.feedexport.S3FeedStorage` " - "without AWS keys is deprecated. Please supply credentials or " - "use the `from_crawler()` constructor.", - category=ScrapyDeprecationWarning, - stacklevel=2 - ) - access_key = settings['AWS_ACCESS_KEY_ID'] - secret_key = settings['AWS_SECRET_ACCESS_KEY'] - # END Backward compatibility + no_keys = access_key is None and secret_key is None + if no_keys: + raise NotConfigured('%s is missing AWS credentials' % + self.__class__.__name__) u = urlparse(uri) self.bucketname = u.hostname self.access_key = u.username or access_key From 98e8086d1b3da8906d1395f453b1299b4e3b499e Mon Sep 17 00:00:00 2001 From: nyov Date: Sat, 7 Mar 2020 19:21:09 +0000 Subject: [PATCH 05/22] Adapt S3FeedStorage testcase --- tests/test_feedexport.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 21da9fdcd..34a67fff3 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -24,6 +24,7 @@ from zope.interface.verify import verifyObject import scrapy from scrapy.crawler import CrawlerRunner +from scrapy.exceptions import NotConfigured from scrapy.exporters import CsvItemExporter from scrapy.extensions.feedexport import ( BlockingFeedStorage, @@ -176,10 +177,6 @@ class BlockingFeedStorageTest(unittest.TestCase): class S3FeedStorageTest(unittest.TestCase): - @mock.patch('scrapy.utils.project.get_project_settings', - new=mock.MagicMock(return_value={'AWS_ACCESS_KEY_ID': 'conf_key', - 'AWS_SECRET_ACCESS_KEY': 'conf_secret'}), - create=True) def test_parse_credentials(self): try: import botocore # noqa: F401 @@ -205,12 +202,9 @@ class S3FeedStorageTest(unittest.TestCase): aws_credentials['AWS_SECRET_ACCESS_KEY']) self.assertEqual(storage.access_key, 'uri_key') self.assertEqual(storage.secret_key, 'uri_secret') - # Backward compatibility for initialising without settings - with warnings.catch_warnings(record=True) as w: - storage = S3FeedStorage('s3://mybucket/export.csv') - self.assertEqual(storage.access_key, 'conf_key') - self.assertEqual(storage.secret_key, 'conf_secret') - self.assertTrue('without AWS keys' in str(w[-1].message)) + # Instantiate without credentials + with self.assertRaises(NotConfigured): + S3FeedStorage('s3://mybucket/export.csv') @defer.inlineCallbacks def test_store(self): From 2829cd4268d22a328acd64a6816c479b1fe21f39 Mon Sep 17 00:00:00 2001 From: nyov Date: Tue, 17 Mar 2020 10:19:13 +0000 Subject: [PATCH 06/22] Allow use without credentials --- scrapy/extensions/feedexport.py | 4 ---- tests/test_feedexport.py | 4 ---- 2 files changed, 8 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index e793c12dc..68d6533d3 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -94,10 +94,6 @@ class FileFeedStorage: class S3FeedStorage(BlockingFeedStorage): def __init__(self, uri, access_key=None, secret_key=None, acl=None): - no_keys = access_key is None and secret_key is None - if no_keys: - raise NotConfigured('%s is missing AWS credentials' % - self.__class__.__name__) u = urlparse(uri) self.bucketname = u.hostname self.access_key = u.username or access_key diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 34a67fff3..656bb515f 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -24,7 +24,6 @@ from zope.interface.verify import verifyObject import scrapy from scrapy.crawler import CrawlerRunner -from scrapy.exceptions import NotConfigured from scrapy.exporters import CsvItemExporter from scrapy.extensions.feedexport import ( BlockingFeedStorage, @@ -202,9 +201,6 @@ class S3FeedStorageTest(unittest.TestCase): aws_credentials['AWS_SECRET_ACCESS_KEY']) self.assertEqual(storage.access_key, 'uri_key') self.assertEqual(storage.secret_key, 'uri_secret') - # Instantiate without credentials - with self.assertRaises(NotConfigured): - S3FeedStorage('s3://mybucket/export.csv') @defer.inlineCallbacks def test_store(self): From 430d22e46ebb67aba39005365e2e68d834d8fb86 Mon Sep 17 00:00:00 2001 From: Artur Shellunts Date: Tue, 21 Jul 2020 23:39:04 +0200 Subject: [PATCH 07/22] Remove not used import warnings --- tests/test_feedexport.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 656bb515f..e80e07554 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -5,7 +5,6 @@ import random import shutil import string import tempfile -import warnings from io import BytesIO from logging import getLogger from pathlib import Path From aefd43a6c6b8d2e32dfb9bd0c94529805ce5037e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 11 Aug 2020 12:52:54 +0200 Subject: [PATCH 08/22] Upgrade minimum dependencies for Python 3.6 support --- tests/requirements-py3.txt | 3 +-- tox.ini | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index 00c56084d..4fa58d11b 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -1,8 +1,7 @@ # Tests requirements attrs dataclasses; python_version == '3.6' -mitmproxy; python_version >= '3.6' -mitmproxy<4.0.0; python_version < '3.6' +mitmproxy >=4, <5 # The latest version does not support some pinned dependencies # https://github.com/pytest-dev/pytest-twisted/issues/93 pytest != 5.4, != 5.4.1 pytest-azurepipelines diff --git a/tox.ini b/tox.ini index 11882c03f..4f5531aea 100644 --- a/tox.ini +++ b/tox.ini @@ -13,7 +13,7 @@ deps = -rtests/requirements-py3.txt # Extras boto3>=1.13.0 - botocore>=1.3.23 + botocore>=1.4.87 Pillow>=3.4.2 passenv = S3_TEST_FILE_URI @@ -76,7 +76,7 @@ deps = zope.interface==4.1.3 -rtests/requirements-py3.txt # Extras - botocore==1.3.23 + botocore==1.4.87 google-cloud-storage==1.29.0 Pillow==3.4.2 From 394631fc0a731a0b8b0108edc1711c0c87e2ca15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 12 Aug 2020 12:08:09 +0200 Subject: [PATCH 09/22] Restore 3.5 support for mitmproxy-based tests --- tests/requirements-py3.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index 4fa58d11b..b67c08403 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -1,7 +1,8 @@ # Tests requirements attrs dataclasses; python_version == '3.6' -mitmproxy >=4, <5 # The latest version does not support some pinned dependencies +mitmproxy >=4, <5; python_version < '3.6' # The latest version does not support some pinned dependencies +mitmproxy <4; python_version < '3.6' # https://github.com/pytest-dev/pytest-twisted/issues/93 pytest != 5.4, != 5.4.1 pytest-azurepipelines From 8e393a0b218b56cc8a70f7bda277969aa026790d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 12 Aug 2020 12:29:51 +0200 Subject: [PATCH 10/22] Do not change the mitmproxy version for no-3.6 Python versions --- tests/requirements-py3.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index b67c08403..b425ce771 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -1,8 +1,9 @@ # Tests requirements attrs -dataclasses; python_version == '3.6' -mitmproxy >=4, <5; python_version < '3.6' # The latest version does not support some pinned dependencies -mitmproxy <4; python_version < '3.6' +dataclasses; python_version ==3.6 +mitmproxy; python_version >=3.7 +mitmproxy >=4, <5; python_version >=3.6, <3.7 +mitmproxy <4; python_version <3.6 # https://github.com/pytest-dev/pytest-twisted/issues/93 pytest != 5.4, != 5.4.1 pytest-azurepipelines From b1de55d37d6a1b84e92df2d93d83e247e5d0e0d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 12 Aug 2020 12:34:40 +0200 Subject: [PATCH 11/22] Fix marker syntax --- tests/requirements-py3.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index b425ce771..b51177abb 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -1,9 +1,9 @@ # Tests requirements attrs -dataclasses; python_version ==3.6 -mitmproxy; python_version >=3.7 -mitmproxy >=4, <5; python_version >=3.6, <3.7 -mitmproxy <4; python_version <3.6 +dataclasses; python_version == '3.6' +mitmproxy; python_version >= '3.7' +mitmproxy >= 4, < 5; python_version >= '3.6' and python_version < '3.7' +mitmproxy < 4; python_version < '3.6' # https://github.com/pytest-dev/pytest-twisted/issues/93 pytest != 5.4, != 5.4.1 pytest-azurepipelines From 756c368a6b0d2eef65f86f8418f9a7fbeff036c7 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 14 Aug 2020 22:09:24 +0500 Subject: [PATCH 12/22] Use a longer key in mitmproxy-ca.pem. --- tests/keys/mitmproxy-ca.pem | 78 +++++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/tests/keys/mitmproxy-ca.pem b/tests/keys/mitmproxy-ca.pem index 08004feca..cdef75f99 100644 --- a/tests/keys/mitmproxy-ca.pem +++ b/tests/keys/mitmproxy-ca.pem @@ -1,32 +1,50 @@ ------BEGIN RSA PRIVATE KEY----- -MIICWwIBAAKBgQDKLbznLxS7HSWvrmGcvVS6eQvjEWD705/csvnk/WtqAPfQMJKt -auFBxzPt6RT60SHtj/2FKt2gqsiE6cNINxGN6fGYD7HtaM5HXRVPUKJaMipJwHha -QivjIZoueraY/MtlyCkpp6dmMnHEpGY7OzwMyh1eCBHQ2JYx6VEzbks9ewIDAQAB -AoGAMpS2ye/Rc+6a2xT5fskvRWe7PZe/d8E+IWz1cACmuuJ7HS7Jw3EV4esAZukF -QqrHnjOD7akHwYZ4nCgPnyWH0lLx/4TIXE5QeLPFrhKOsSLCyhlCwNVJAdcOrDol -Qh2694Dsd4gAy5o6TA02cBpqArnbAUERX46bHBZRA+ths8ECQQD6r1Ls+bTBR52w -T3rPPhYj7EsXp40MJt0pLf1kjf+EH1bxsUqnxLawwo/lLE9omU73DFnfrAflk2Ll -KUPCjjYpAkEAznchXk2ITeRcClrBNA+1Izpb5yG1qkfc79u/CEVDDOvt7RO/89Oj -58R3pKTyffoo34fBdJz8GYDsmOeiyJEjAwJANpSHrJrtlQt/tMyJQ6gT7/xZmSvc -1OF9U6L0wbj9AgpExtjAFWkKEdA6vj34iCChBb8FrmJpUb3WUWi7nReTiQJAFyIT -9Av93LRcd7CJezrTUdolF/WX9DdPEvTtJ5ETHSyGIQ0Yccph0AMcYK82mFTiJYGB -dH5uZLEkUVGK1KwmXwJAGWLdYiQyQitRWdoURcLb4OZ2gF3+7PASgFilI8YuoYhn -Rl2Va3UtErPKJeMg2dTH18PuXykQMsQR1+rPxf1WSA== ------END RSA PRIVATE KEY----- +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQCYp6U4G9YWITYB +/JlZ+Hd08c/9a157WVl03hbR2DSK8FnK+D8cp2dGzuTfC08w8M/yvVYPcbb7ZDiT +NUsVwboFvmr/6mN6M9uQioCRStrP6Rkm2Wuagyj+GjqLwogTJlPiPwEPhlMgz1BJ +u6jQQSgiMsxKWMkVz3pCYERUMRX0DEgYST9rjYUAwD4rPv8XXtLLSPs0VniIggUH +JrngDUrtoK5Wuf098NJPIwW8uE2ev+DXH2Iuwn2fNKt5lSYypJdUZjyamwuE6HFB +eIBAIIKijMz/8UV1+H8Q0OcU2Sva2FglHREQtA/S5FlpcuTZt/77Vnxv75y/0zls +90iyQ3E/AgMBAAECggEBAJA1dyAdM85uC04vKVNUJM1GDp0xS+0syBReJaKRI3nJ +epoCj+RqxGag1pdaYLI0G84NTPqECz9LOyLdqpPgEfKRIxWlf9oWmSnfnXskArd8 +VfVcWYl6tEPv1TToTZIBmCbYLBFVbLxG/GrbK6uokdhUsqbdXwEKok2IEaSTRlDn +v8BVXte00d9VEKKpmI6EY3f45uPQPHuJNcitP2HGW1mT/C6XoZR6wj+VvoRgUGQT +I7PuktbYpQlLV+oX0uZz9frPGhjydUq0Jti5v3QAJEb+7D0cKrkZW+7fYDx4YkRU +oDiuWEyO2kfpff52Qxs+xUXMiAyw6/8+TamKoAi1TIECgYEAyAzoztW6W4CjL2au +/hN5VmbAvuBxq1m1G5KgXM1myX9V2CgH6OKwzJQNSCEfKMNOjqxB99T7C3tMCjgG +gmbUzylTeciQFF+crrl2Rn/6qZS9dCo1hagb3K5eXMhLXoP425Y4sypNPPqULhPn +YrUDFNAf89rRLqP1KMPLZ+uO7EECgYEAw1lWPxGV+X85iQxYN9xoX85htfJSBXTf +dLirQ4bkykOxSA6ZzFuhDO/G373Q1rze4tmEO790uOCeaiXGgeWC1A+2PMO957i5 +9FqhDIkmerfdIttdEUMM9rQwuTcLnixGZkT5GHDzjtNinaIVB+pv7twRAESqN9dC +QXh7IF7g/X8CgYBMhQOX+hCqZ24D95cAAJrs/ajEWj2geVPZFCDa3oZulJJVeBpu +bieKWScra9/rS6mE0Ub6cTEFl0fisMNspcDI7NnNP3Y9FMVt3+rp1JIgw5AkGvEW +CtN9egUGIGcT5A8Qj0lo3slkhcSgS2S6UNq431MZh51z5askyJ/JREULAQKBgFrR +OatwfYzUfOcd+hVePpfr1rlDwqYOw6P8BoMKP2tZNR4Oy6maH7Fn98kk8eYjQGuu +PC+avqUEqCEpFrRlAwGbnFl7ltoXozvatmyhhmYe/Iur+ASCa5B2DQDOenQ6mTAK +eNPIDzMjSwGFzMk1UHx3it/ZDFmRlZfibzuJYIf5AoGBAIaPHk4qadK/XpcD4Wwx +BOsDEIz27DGWdwWfd5r3EcV4zX/wNzH0G1Z8eydNjUqKzufMZgFwpcTu0Evesl1/ +B8kC8sLHxQoG5SvBu4dBxMwKIU9O9uFnX5SUYZUDpCtUYyZ+GtGom41Jwg5ENrwy +HzPh2taMnCA0h1fNLFFBkw88 +-----END PRIVATE KEY----- -----BEGIN CERTIFICATE----- -MIICnzCCAgigAwIBAgIGDI2K/EOjMA0GCSqGSIb3DQEBBQUAMCgxEjAQBgNVBAMT -CW1pdG1wcm94eTESMBAGA1UEChMJbWl0bXByb3h5MB4XDTEzMDkyNjE0MzYxMVoX -DTE1MDkxNjE0MzYxMVowKDESMBAGA1UEAxMJbWl0bXByb3h5MRIwEAYDVQQKEwlt -aXRtcHJveHkwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMotvOcvFLsdJa+u -YZy9VLp5C+MRYPvTn9yy+eT9a2oA99Awkq1q4UHHM+3pFPrRIe2P/YUq3aCqyITp -w0g3EY3p8ZgPse1ozkddFU9QoloyKknAeFpCK+Mhmi56tpj8y2XIKSmnp2YyccSk -Zjs7PAzKHV4IEdDYljHpUTNuSz17AgMBAAGjgdMwgdAwDwYDVR0TAQH/BAUwAwEB -/zAUBglghkgBhvhCAQEBAf8EBAMCAgQwewYDVR0lAQH/BHEwbwYIKwYBBQUHAwEG -CCsGAQUFBwMCBggrBgEFBQcDBAYIKwYBBQUHAwgGCisGAQQBgjcCARUGCisGAQQB -gjcCARYGCisGAQQBgjcKAwEGCisGAQQBgjcKAwMGCisGAQQBgjcKAwQGCWCGSAGG -+EIEATALBgNVHQ8EBAMCAQYwHQYDVR0OBBYEFJBEfawVwhEHHW6rS8nvZFlJ582n -MA0GCSqGSIb3DQEBBQUAA4GBAHGl28Ip2CWS/MibCaFztLDxGiMBT4MW2yI2hf3D -y9g1o7ra/fSEFdIc849xXyCsGWSkMsbDML272rCH4K73MUBxxkJm46AIyRVH1z2Z -e96u4py1wNT8cznY15phr8pn36snlaHaYa+JcwGINMdSOk1VPHv6gqSC/vgUCgF1 -n95u +MIIDoTCCAomgAwIBAgIGDodLQx9+MA0GCSqGSIb3DQEBCwUAMCgxEjAQBgNVBAMM +CW1pdG1wcm94eTESMBAGA1UECgwJbWl0bXByb3h5MB4XDTIwMDgxMjE3MDMyNloX +DTIzMDgxNDE3MDMyNlowKDESMBAGA1UEAwwJbWl0bXByb3h5MRIwEAYDVQQKDAlt +aXRtcHJveHkwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCYp6U4G9YW +ITYB/JlZ+Hd08c/9a157WVl03hbR2DSK8FnK+D8cp2dGzuTfC08w8M/yvVYPcbb7 +ZDiTNUsVwboFvmr/6mN6M9uQioCRStrP6Rkm2Wuagyj+GjqLwogTJlPiPwEPhlMg +z1BJu6jQQSgiMsxKWMkVz3pCYERUMRX0DEgYST9rjYUAwD4rPv8XXtLLSPs0VniI +ggUHJrngDUrtoK5Wuf098NJPIwW8uE2ev+DXH2Iuwn2fNKt5lSYypJdUZjyamwuE +6HFBeIBAIIKijMz/8UV1+H8Q0OcU2Sva2FglHREQtA/S5FlpcuTZt/77Vnxv75y/ +0zls90iyQ3E/AgMBAAGjgdAwgc0wDwYDVR0TAQH/BAUwAwEB/zARBglghkgBhvhC +AQEEBAMCAgQweAYDVR0lBHEwbwYIKwYBBQUHAwEGCCsGAQUFBwMCBggrBgEFBQcD +BAYIKwYBBQUHAwgGCisGAQQBgjcCARUGCisGAQQBgjcCARYGCisGAQQBgjcKAwEG +CisGAQQBgjcKAwMGCisGAQQBgjcKAwQGCWCGSAGG+EIEATAOBgNVHQ8BAf8EBAMC +AQYwHQYDVR0OBBYEFBCsLPpFz3l9rOOfGmfs+VRc3jhJMA0GCSqGSIb3DQEBCwUA +A4IBAQADTpA15na6U5qqDCe0rr39fkS1/dY804Xnz7g/L3AsxPE1KOMijuJa8sKd +kKwba1173FwMupfK39zY8jUxL8Qprdi92RO6CpoFUsL/icpA///lYhzUSqt32qwe +gRNW3mtYBimOk6KH1NOfQnJolWpJh+g1OEsitQKEeKwIn5Hz+8/yS5tbwLgdnMlY +1/it1H70JSdE7nfJueqN4cFfBsm6XaHZzacJJmN7WP88fd+zztnSQsBFbLlnjnqj +envCDIwCrMywKNMqEBMwmBEGSAF47fVNYj6KzDAtMvBdDkYaHWpBf4tnFfk6v0wj +wiKjdLjCmJgjGAQjRw5VYJ8JI0XO -----END CERTIFICATE----- From 2aa4f3cbf96ad55aa4a1fa064758338daf16b110 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <1731933+elacuesta@users.noreply.github.com> Date: Mon, 17 Aug 2020 05:39:59 -0300 Subject: [PATCH 13/22] Conditional request attribute binding for responses (#4632) --- docs/topics/signals.rst | 5 + scrapy/core/engine.py | 13 +- scrapy/core/scraper.py | 62 ++++---- setup.cfg | 3 + tests/test_request_attribute_binding.py | 202 ++++++++++++++++++++++++ 5 files changed, 250 insertions(+), 35 deletions(-) create mode 100644 tests/test_request_attribute_binding.py diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 255ba9d3f..1d99d8c28 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -423,6 +423,11 @@ response_received :param spider: the spider for which the response is intended :type spider: :class:`~scrapy.spiders.Spider` object +.. note:: The ``request`` argument might not contain the original request that + reached the downloader, if a :ref:`topics-downloader-middleware` modifies + the :class:`~scrapy.http.Response` object and sets a specific ``request`` + attribute. + response_downloaded ~~~~~~~~~~~~~~~~~~~ diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 86a6abb23..5e0dfe37c 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -243,12 +243,17 @@ class ExecutionEngine: % (type(response), response) ) if isinstance(response, Response): - response.request = request # tie request to response received - logkws = self.logformatter.crawled(request, response, spider) + if response.request is None: + response.request = request + logkws = self.logformatter.crawled(response.request, response, spider) if logkws is not None: logger.log(*logformatter_adapter(logkws), extra={'spider': spider}) - self.signals.send_catch_log(signals.response_received, - response=response, request=request, spider=spider) + self.signals.send_catch_log( + signal=signals.response_received, + response=response, + request=response.request, + spider=spider, + ) return response def _on_complete(_): diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 1ef0790a9..20bdb22a1 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -12,7 +12,7 @@ from scrapy import signals from scrapy.core.spidermw import SpiderMiddlewareManager from scrapy.exceptions import CloseSpider, DropItem, IgnoreRequest from scrapy.http import Request, Response -from scrapy.utils.defer import defer_result, defer_succeed, iter_errback, parallel +from scrapy.utils.defer import defer_fail, defer_succeed, iter_errback, parallel from scrapy.utils.log import failure_to_exc_info, logformatter_adapter from scrapy.utils.misc import load_object, warn_on_generator_with_return_value from scrapy.utils.spider import iterate_spider_output @@ -120,40 +120,40 @@ class Scraper: response, request, deferred = slot.next_response_request_deferred() self._scrape(response, request, spider).chainDeferred(deferred) - def _scrape(self, response, request, spider): - """Handle the downloaded response or failure through the spider - callback/errback""" - if not isinstance(response, (Response, Failure)): - raise TypeError( - "Incorrect type: expected Response or Failure, got %s: %r" - % (type(response), response) - ) - - dfd = self._scrape2(response, request, spider) # returns spider's processed output - dfd.addErrback(self.handle_spider_error, request, response, spider) - dfd.addCallback(self.handle_spider_output, request, response, spider) + def _scrape(self, result, request, spider): + """ + Handle the downloaded response or failure through the spider callback/errback + """ + if not isinstance(result, (Response, Failure)): + raise TypeError("Incorrect type: expected Response or Failure, got %s: %r" % (type(result), result)) + dfd = self._scrape2(result, request, spider) # returns spider's processed output + dfd.addErrback(self.handle_spider_error, request, result, spider) + dfd.addCallback(self.handle_spider_output, request, result, spider) return dfd - def _scrape2(self, request_result, request, spider): - """Handle the different cases of request's result been a Response or a - Failure""" - if not isinstance(request_result, Failure): - return self.spidermw.scrape_response( - self.call_spider, request_result, request, spider) - else: - dfd = self.call_spider(request_result, request, spider) - return dfd.addErrback( - self._log_download_errors, request_result, request, spider) + def _scrape2(self, result, request, spider): + """ + Handle the different cases of request's result been a Response or a Failure + """ + if isinstance(result, Response): + return self.spidermw.scrape_response(self.call_spider, result, request, spider) + else: # result is a Failure + dfd = self.call_spider(result, request, spider) + return dfd.addErrback(self._log_download_errors, result, request, spider) def call_spider(self, result, request, spider): - result.request = request - dfd = defer_result(result) - callback = request.callback or spider._parse - warn_on_generator_with_return_value(spider, callback) - warn_on_generator_with_return_value(spider, request.errback) - dfd.addCallbacks(callback=callback, - errback=request.errback, - callbackKeywords=request.cb_kwargs) + if isinstance(result, Response): + if getattr(result, "request", None) is None: + result.request = request + callback = result.request.callback or spider._parse + warn_on_generator_with_return_value(spider, callback) + dfd = defer_succeed(result) + dfd.addCallback(callback, **result.request.cb_kwargs) + else: # result is a Failure + result.request = request + warn_on_generator_with_return_value(spider, request.errback) + dfd = defer_fail(result) + dfd.addErrback(request.errback) return dfd.addCallback(iterate_spider_output) def handle_spider_error(self, _failure, request, response, spider): diff --git a/setup.cfg b/setup.cfg index f8e7c0c91..3a624ec94 100644 --- a/setup.cfg +++ b/setup.cfg @@ -130,6 +130,9 @@ ignore_errors = True [mypy-tests.test_pipelines] ignore_errors = True +[mypy-tests.test_request_attribute_binding] +ignore_errors = True + [mypy-tests.test_request_cb_kwargs] ignore_errors = True diff --git a/tests/test_request_attribute_binding.py b/tests/test_request_attribute_binding.py new file mode 100644 index 000000000..b60b7c579 --- /dev/null +++ b/tests/test_request_attribute_binding.py @@ -0,0 +1,202 @@ +from twisted.internet import defer +from twisted.trial.unittest import TestCase + +from scrapy import Request, signals +from scrapy.crawler import CrawlerRunner +from scrapy.http.response import Response + +from testfixtures import LogCapture + +from tests.mockserver import MockServer +from tests.spiders import SingleRequestSpider + + +OVERRIDEN_URL = "https://example.org" + + +class ProcessResponseMiddleware: + def process_response(self, request, response, spider): + return response.replace(request=Request(OVERRIDEN_URL)) + + +class RaiseExceptionRequestMiddleware: + def process_request(self, request, spider): + 1 / 0 + return request + + +class CatchExceptionOverrideRequestMiddleware: + def process_exception(self, request, exception, spider): + return Response( + url="http://localhost/", + body=b"Caught " + exception.__class__.__name__.encode("utf-8"), + request=Request(OVERRIDEN_URL), + ) + + +class CatchExceptionDoNotOverrideRequestMiddleware: + def process_exception(self, request, exception, spider): + return Response( + url="http://localhost/", + body=b"Caught " + exception.__class__.__name__.encode("utf-8"), + ) + + +class AlternativeCallbacksSpider(SingleRequestSpider): + name = "alternative_callbacks_spider" + + def alt_callback(self, response, foo=None): + self.logger.info("alt_callback was invoked with foo=%s", foo) + + +class AlternativeCallbacksMiddleware: + def process_response(self, request, response, spider): + new_request = request.replace( + url=OVERRIDEN_URL, + callback=spider.alt_callback, + cb_kwargs={"foo": "bar"}, + ) + return response.replace(request=new_request) + + +class CrawlTestCase(TestCase): + + def setUp(self): + self.mockserver = MockServer() + self.mockserver.__enter__() + + def tearDown(self): + self.mockserver.__exit__(None, None, None) + + @defer.inlineCallbacks + def test_response_200(self): + url = self.mockserver.url("/status?n=200") + crawler = CrawlerRunner().create_crawler(SingleRequestSpider) + yield crawler.crawl(seed=url, mockserver=self.mockserver) + response = crawler.spider.meta["responses"][0] + self.assertEqual(response.request.url, url) + + @defer.inlineCallbacks + def test_response_error(self): + for status in ("404", "500"): + url = self.mockserver.url("/status?n={}".format(status)) + crawler = CrawlerRunner().create_crawler(SingleRequestSpider) + yield crawler.crawl(seed=url, mockserver=self.mockserver) + failure = crawler.spider.meta["failure"] + response = failure.value.response + self.assertEqual(failure.request.url, url) + self.assertEqual(response.request.url, url) + + @defer.inlineCallbacks + def test_downloader_middleware_raise_exception(self): + url = self.mockserver.url("/status?n=200") + runner = CrawlerRunner(settings={ + "DOWNLOADER_MIDDLEWARES": { + __name__ + ".RaiseExceptionRequestMiddleware": 590, + }, + }) + crawler = runner.create_crawler(SingleRequestSpider) + yield crawler.crawl(seed=url, mockserver=self.mockserver) + failure = crawler.spider.meta["failure"] + self.assertEqual(failure.request.url, url) + self.assertIsInstance(failure.value, ZeroDivisionError) + + @defer.inlineCallbacks + def test_downloader_middleware_override_request_in_process_response(self): + """ + Downloader middleware which returns a response with an specific 'request' attribute. + + * The spider callback should receive the overriden response.request + * Handlers listening to the response_received signal should receive the overriden response.request + * The "crawled" log message should show the overriden response.request + """ + signal_params = {} + + def signal_handler(response, request, spider): + signal_params["response"] = response + signal_params["request"] = request + + url = self.mockserver.url("/status?n=200") + runner = CrawlerRunner(settings={ + "DOWNLOADER_MIDDLEWARES": { + __name__ + ".ProcessResponseMiddleware": 595, + } + }) + crawler = runner.create_crawler(SingleRequestSpider) + crawler.signals.connect(signal_handler, signal=signals.response_received) + + with LogCapture() as log: + yield crawler.crawl(seed=url, mockserver=self.mockserver) + + response = crawler.spider.meta["responses"][0] + self.assertEqual(response.request.url, OVERRIDEN_URL) + + self.assertEqual(signal_params["response"].url, url) + self.assertEqual(signal_params["request"].url, OVERRIDEN_URL) + + log.check_present( + ("scrapy.core.engine", "DEBUG", "Crawled (200) (referer: None)".format(OVERRIDEN_URL)), + ) + + @defer.inlineCallbacks + def test_downloader_middleware_override_in_process_exception(self): + """ + An exception is raised but caught by the next middleware, which + returns a Response with a specific 'request' attribute. + + The spider callback should receive the overriden response.request + """ + url = self.mockserver.url("/status?n=200") + runner = CrawlerRunner(settings={ + "DOWNLOADER_MIDDLEWARES": { + __name__ + ".RaiseExceptionRequestMiddleware": 590, + __name__ + ".CatchExceptionOverrideRequestMiddleware": 595, + }, + }) + crawler = runner.create_crawler(SingleRequestSpider) + yield crawler.crawl(seed=url, mockserver=self.mockserver) + response = crawler.spider.meta["responses"][0] + self.assertEqual(response.body, b"Caught ZeroDivisionError") + self.assertEqual(response.request.url, OVERRIDEN_URL) + + @defer.inlineCallbacks + def test_downloader_middleware_do_not_override_in_process_exception(self): + """ + An exception is raised but caught by the next middleware, which + returns a Response without a specific 'request' attribute. + + The spider callback should receive the original response.request + """ + url = self.mockserver.url("/status?n=200") + runner = CrawlerRunner(settings={ + "DOWNLOADER_MIDDLEWARES": { + __name__ + ".RaiseExceptionRequestMiddleware": 590, + __name__ + ".CatchExceptionDoNotOverrideRequestMiddleware": 595, + }, + }) + crawler = runner.create_crawler(SingleRequestSpider) + yield crawler.crawl(seed=url, mockserver=self.mockserver) + response = crawler.spider.meta["responses"][0] + self.assertEqual(response.body, b"Caught ZeroDivisionError") + self.assertEqual(response.request.url, url) + + @defer.inlineCallbacks + def test_downloader_middleware_alternative_callback(self): + """ + Downloader middleware which returns a response with a + specific 'request' attribute, with an alternative callback + """ + runner = CrawlerRunner(settings={ + "DOWNLOADER_MIDDLEWARES": { + __name__ + ".AlternativeCallbacksMiddleware": 595, + } + }) + crawler = runner.create_crawler(AlternativeCallbacksSpider) + + with LogCapture() as log: + url = self.mockserver.url("/status?n=200") + yield crawler.crawl(seed=url, mockserver=self.mockserver) + + log.check_present( + ("alternative_callbacks_spider", "INFO", "alt_callback was invoked with foo=bar"), + ) From a8e08d51cd50e8f0b58a60992d310e56d4f71641 Mon Sep 17 00:00:00 2001 From: Ajay Mittur Date: Mon, 17 Aug 2020 14:15:52 +0530 Subject: [PATCH 14/22] Check if file is already present on running `scrapy genspider` and terminate if so (#4623) --- scrapy/commands/genspider.py | 41 ++++++++++++++++------ tests/test_commands.py | 67 +++++++++++++++++++++++++++++++++++- 2 files changed, 97 insertions(+), 11 deletions(-) diff --git a/scrapy/commands/genspider.py b/scrapy/commands/genspider.py index 4c7548e9c..74a077d1b 100644 --- a/scrapy/commands/genspider.py +++ b/scrapy/commands/genspider.py @@ -66,16 +66,9 @@ class Command(ScrapyCommand): print("Cannot create a spider with the same name as your project") return - try: - spidercls = self.crawler_process.spider_loader.load(name) - except KeyError: - pass - else: - # if spider already exists and not --force then halt - if not opts.force: - print("Spider %r already exists in module:" % name) - print(" %s" % spidercls.__module__) - return + if not opts.force and self._spider_exists(name): + return + template_file = self._find_template(opts.template) if template_file: self._genspider(module, name, domain, opts.template, template_file) @@ -119,6 +112,34 @@ class Command(ScrapyCommand): if filename.endswith('.tmpl'): print(" %s" % splitext(filename)[0]) + def _spider_exists(self, name): + if not self.settings.get('NEWSPIDER_MODULE'): + # if run as a standalone command and file with same filename already exists + if exists(name + ".py"): + print("%s already exists" % (abspath(name + ".py"))) + return True + return False + + try: + spidercls = self.crawler_process.spider_loader.load(name) + except KeyError: + pass + else: + # if spider with same name exists + print("Spider %r already exists in module:" % name) + print(" %s" % spidercls.__module__) + return True + + # a file with the same name exists in the target directory + spiders_module = import_module(self.settings['NEWSPIDER_MODULE']) + spiders_dir = dirname(spiders_module.__file__) + spiders_dir_abs = abspath(spiders_dir) + if exists(join(spiders_dir_abs, name + ".py")): + print("%s already exists" % (join(spiders_dir_abs, (name + ".py")))) + return True + + return False + @property def templates_dir(self): return join( diff --git a/tests/test_commands.py b/tests/test_commands.py index 8938156fc..109f006a8 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -8,7 +8,7 @@ import sys import tempfile from contextlib import contextmanager from itertools import chain -from os.path import exists, join, abspath +from os.path import exists, join, abspath, getmtime from pathlib import Path from shutil import rmtree, copytree from stat import S_IWRITE as ANYONE_WRITE_PERMISSION @@ -337,8 +337,11 @@ class GenspiderCommandTest(CommandTest): p, out, err = self.proc('genspider', spname, 'test.com', *args) self.assertIn("Created spider %r using template %r in module" % (spname, tplname), out) self.assertTrue(exists(join(self.proj_mod_path, 'spiders', 'test_spider.py'))) + modify_time_before = getmtime(join(self.proj_mod_path, 'spiders', 'test_spider.py')) p, out, err = self.proc('genspider', spname, 'test.com', *args) self.assertIn("Spider %r already exists in module" % spname, out) + modify_time_after = getmtime(join(self.proj_mod_path, 'spiders', 'test_spider.py')) + self.assertEqual(modify_time_after, modify_time_before) def test_template_basic(self): self.test_template('basic') @@ -360,6 +363,40 @@ class GenspiderCommandTest(CommandTest): self.assertEqual(2, self.call('genspider', self.project_name)) assert not exists(join(self.proj_mod_path, 'spiders', '%s.py' % self.project_name)) + def test_same_filename_as_existing_spider(self, force=False): + file_name = 'example' + file_path = join(self.proj_mod_path, 'spiders', '%s.py' % file_name) + self.assertEqual(0, self.call('genspider', file_name, 'example.com')) + assert exists(file_path) + + # change name of spider but not its file name + with open(file_path, 'r+') as spider_file: + file_data = spider_file.read() + file_data = file_data.replace("name = \'example\'", "name = \'renamed\'") + spider_file.seek(0) + spider_file.write(file_data) + spider_file.truncate() + modify_time_before = getmtime(file_path) + file_contents_before = file_data + + if force: + p, out, err = self.proc('genspider', '--force', file_name, 'example.com') + self.assertIn("Created spider %r using template \'basic\' in module" % file_name, out) + modify_time_after = getmtime(file_path) + self.assertNotEqual(modify_time_after, modify_time_before) + file_contents_after = open(file_path, 'r').read() + self.assertNotEqual(file_contents_after, file_contents_before) + else: + p, out, err = self.proc('genspider', file_name, 'example.com') + self.assertIn("%s already exists" % (file_path), out) + modify_time_after = getmtime(file_path) + self.assertEqual(modify_time_after, modify_time_before) + file_contents_after = open(file_path, 'r').read() + self.assertEqual(file_contents_after, file_contents_before) + + def test_same_filename_as_existing_spider_force(self): + self.test_same_filename_as_existing_spider(force=True) + class GenspiderStandaloneCommandTest(ProjectTest): @@ -367,6 +404,34 @@ class GenspiderStandaloneCommandTest(ProjectTest): self.call('genspider', 'example', 'example.com') assert exists(join(self.temp_path, 'example.py')) + def test_same_name_as_existing_file(self, force=False): + file_name = 'example' + file_path = join(self.temp_path, file_name + '.py') + p, out, err = self.proc('genspider', file_name, 'example.com') + self.assertIn("Created spider %r using template \'basic\' " % file_name, out) + assert exists(file_path) + modify_time_before = getmtime(file_path) + file_contents_before = open(file_path, 'r').read() + + if force: + # use different template to ensure contents were changed + p, out, err = self.proc('genspider', '--force', '-t', 'crawl', file_name, 'example.com') + self.assertIn("Created spider %r using template \'crawl\' " % file_name, out) + modify_time_after = getmtime(file_path) + self.assertNotEqual(modify_time_after, modify_time_before) + file_contents_after = open(file_path, 'r').read() + self.assertNotEqual(file_contents_after, file_contents_before) + else: + p, out, err = self.proc('genspider', file_name, 'example.com') + self.assertIn("%s already exists" % join(self.temp_path, file_name + ".py"), out) + modify_time_after = getmtime(file_path) + self.assertEqual(modify_time_after, modify_time_before) + file_contents_after = open(file_path, 'r').read() + self.assertEqual(file_contents_after, file_contents_before) + + def test_same_name_as_existing_file_force(self): + self.test_same_name_as_existing_file(force=True) + class MiscCommandsTest(CommandTest): From 55edf8d3b8885541cdbf9d1c62d9a6bbf634e2a0 Mon Sep 17 00:00:00 2001 From: Grammy Jiang <719388+grammy-jiang@users.noreply.github.com> Date: Mon, 17 Aug 2020 18:50:52 +1000 Subject: [PATCH 15/22] Add typing hint to httpcache downloadermiddlewares (#4243) --- scrapy/downloadermiddlewares/httpcache.py | 41 ++++++++++++++++------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcache.py b/scrapy/downloadermiddlewares/httpcache.py index 6db57bd8b..62f1c3a29 100644 --- a/scrapy/downloadermiddlewares/httpcache.py +++ b/scrapy/downloadermiddlewares/httpcache.py @@ -1,4 +1,5 @@ from email.utils import formatdate +from typing import Optional, Type, TypeVar from twisted.internet import defer from twisted.internet.error import ( @@ -13,10 +14,19 @@ from twisted.internet.error import ( from twisted.web.client import ResponseFailed from scrapy import signals +from scrapy.crawler import Crawler from scrapy.exceptions import IgnoreRequest, NotConfigured +from scrapy.http.request import Request +from scrapy.http.response import Response +from scrapy.settings import Settings +from scrapy.spiders import Spider +from scrapy.statscollectors import StatsCollector from scrapy.utils.misc import load_object +HttpCacheMiddlewareTV = TypeVar("HttpCacheMiddlewareTV", bound="HttpCacheMiddleware") + + class HttpCacheMiddleware: DOWNLOAD_EXCEPTIONS = (defer.TimeoutError, TimeoutError, DNSLookupError, @@ -24,7 +34,7 @@ class HttpCacheMiddleware: ConnectionLost, TCPTimedOutError, ResponseFailed, IOError) - def __init__(self, settings, stats): + def __init__(self, settings: Settings, stats: StatsCollector) -> None: if not settings.getbool('HTTPCACHE_ENABLED'): raise NotConfigured self.policy = load_object(settings['HTTPCACHE_POLICY'])(settings) @@ -33,26 +43,26 @@ class HttpCacheMiddleware: self.stats = stats @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls: Type[HttpCacheMiddlewareTV], crawler: Crawler) -> HttpCacheMiddlewareTV: o = cls(crawler.settings, crawler.stats) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) return o - def spider_opened(self, spider): + def spider_opened(self, spider: Spider) -> None: self.storage.open_spider(spider) - def spider_closed(self, spider): + def spider_closed(self, spider: Spider) -> None: self.storage.close_spider(spider) - def process_request(self, request, spider): + def process_request(self, request: Request, spider: Spider) -> Optional[Response]: if request.meta.get('dont_cache', False): - return + return None # Skip uncacheable requests if not self.policy.should_cache_request(request): request.meta['_dont_cache'] = True # flag as uncacheable - return + return None # Look for cached response and check if expired cachedresponse = self.storage.retrieve_response(spider, request) @@ -61,7 +71,7 @@ class HttpCacheMiddleware: if self.ignore_missing: self.stats.inc_value('httpcache/ignore', spider=spider) raise IgnoreRequest("Ignored request not in cache: %s" % request) - return # first time request + return None # first time request # Return cached response only if not expired cachedresponse.flags.append('cached') @@ -73,7 +83,9 @@ class HttpCacheMiddleware: # process_response hook request.meta['cached_response'] = cachedresponse - def process_response(self, request, response, spider): + return None + + def process_response(self, request: Request, response: Response, spider: Spider) -> Response: if request.meta.get('dont_cache', False): return response @@ -85,7 +97,7 @@ class HttpCacheMiddleware: # RFC2616 requires origin server to set Date header, # https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.18 if 'Date' not in response.headers: - response.headers['Date'] = formatdate(usegmt=1) + response.headers['Date'] = formatdate(usegmt=True) # Do not validate first-hand responses cachedresponse = request.meta.pop('cached_response', None) @@ -102,13 +114,18 @@ class HttpCacheMiddleware: self._cache_response(spider, response, request, cachedresponse) return response - def process_exception(self, request, exception, spider): + def process_exception( + self, request: Request, exception: Exception, spider: Spider + ) -> Optional[Response]: cachedresponse = request.meta.pop('cached_response', None) if cachedresponse is not None and isinstance(exception, self.DOWNLOAD_EXCEPTIONS): self.stats.inc_value('httpcache/errorrecovery', spider=spider) return cachedresponse + return None - def _cache_response(self, spider, response, request, cachedresponse): + def _cache_response( + self, spider: Spider, response: Response, request: Request, cachedresponse: Optional[Response] + ) -> None: if self.policy.should_cache_response(response, request): self.stats.inc_value('httpcache/store', spider=spider) self.storage.store_response(spider, request, response) From e70975f0bb4e8378bf0e00ae7a7014247cd59441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 17 Aug 2020 15:10:08 +0200 Subject: [PATCH 16/22] Allow overwriting feeds (#4512) Co-authored-by: Yuval Hager --- docs/faq.rst | 6 +- docs/intro/overview.rst | 26 +- docs/intro/tutorial.rst | 13 +- docs/topics/feed-exports.rst | 47 +++- scrapy/commands/__init__.py | 15 +- scrapy/extensions/feedexport.py | 142 +++++++--- scrapy/utils/conf.py | 44 +++- scrapy/utils/ftp.py | 6 +- scrapy/utils/python.py | 3 +- tests/ftpserver.py | 24 ++ tests/mockserver.py | 26 ++ tests/requirements-py3.txt | 1 + tests/test_commands.py | 126 +++++++++ tests/test_feedexport.py | 442 ++++++++++++++++++++++++++++---- tests/test_utils_conf.py | 16 ++ tests/test_utils_python.py | 4 + 16 files changed, 791 insertions(+), 150 deletions(-) create mode 100644 tests/ftpserver.py diff --git a/docs/faq.rst b/docs/faq.rst index ea2c8216f..9346ec358 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -236,15 +236,15 @@ Simplest way to dump all my scraped items into a JSON/CSV/XML file? To dump into a JSON file:: - scrapy crawl myspider -o items.json + scrapy crawl myspider -O items.json To dump into a CSV file:: - scrapy crawl myspider -o items.csv + scrapy crawl myspider -O items.csv To dump into a XML file:: - scrapy crawl myspider -o items.xml + scrapy crawl myspider -O items.xml For more information see :ref:`topics-feed-exports` diff --git a/docs/intro/overview.rst b/docs/intro/overview.rst index 01986b594..dd80c7bd0 100644 --- a/docs/intro/overview.rst +++ b/docs/intro/overview.rst @@ -42,30 +42,18 @@ http://quotes.toscrape.com, following the pagination:: if next_page is not None: yield response.follow(next_page, self.parse) - Put this in a text file, name it to something like ``quotes_spider.py`` and run the spider using the :command:`runspider` command:: - scrapy runspider quotes_spider.py -o quotes.json + scrapy runspider quotes_spider.py -o quotes.jl +When this finishes you will have in the ``quotes.jl`` file a list of the +quotes in JSON Lines format, containing text and author, looking like this:: -When this finishes you will have in the ``quotes.json`` file a list of the -quotes in JSON format, containing text and author, looking like this (reformatted -here for better readability):: - - [{ - "author": "Jane Austen", - "text": "\u201cThe person, be it gentleman or lady, who has not pleasure in a good novel, must be intolerably stupid.\u201d" - }, - { - "author": "Groucho Marx", - "text": "\u201cOutside of a dog, a book is man's best friend. Inside of a dog it's too dark to read.\u201d" - }, - { - "author": "Steve Martin", - "text": "\u201cA day without sunshine is like, you know, night.\u201d" - }, - ...] + {"author": "Jane Austen", "text": "\u201cThe person, be it gentleman or lady, who has not pleasure in a good novel, must be intolerably stupid.\u201d"} + {"author": "Steve Martin", "text": "\u201cA day without sunshine is like, you know, night.\u201d"} + {"author": "Garrison Keillor", "text": "\u201cAnyone who thinks sitting in church can make you a Christian must also think that sitting in a garage can make you a car.\u201d"} + ... What just happened? diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index 5f35dc936..f96c78887 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -464,16 +464,15 @@ Storing the scraped data The simplest way to store the scraped data is by using :ref:`Feed exports `, with the following command:: - scrapy crawl quotes -o quotes.json + scrapy crawl quotes -O quotes.json That will generate an ``quotes.json`` file containing all scraped items, serialized in `JSON`_. -For historic reasons, Scrapy appends to a given file instead of overwriting -its contents. If you run this command twice without removing the file -before the second time, you'll end up with a broken JSON file. - -You can also use other formats, like `JSON Lines`_:: +The ``-O`` command-line switch overwrites any existing file; use ``-o`` instead +to append new content to any existing file. However, appending to a JSON file +makes the file contents invalid JSON. When appending to a file, consider +using a different serialization format, such as `JSON Lines`_:: scrapy crawl quotes -o quotes.jl @@ -704,7 +703,7 @@ Using spider arguments You can provide command line arguments to your spiders by using the ``-a`` option when running them:: - scrapy crawl quotes -o quotes-humor.json -a tag=humor + scrapy crawl quotes -O quotes-humor.json -a tag=humor These arguments are passed to the Spider's ``__init__`` method and become spider attributes by default. diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 0f0f258dc..cd4f7cf29 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -291,6 +291,7 @@ Default: ``{}`` A dictionary in which every key is a feed URI (or a :class:`pathlib.Path` object) and each value is a nested dictionary containing configuration parameters for the specific feed. + This setting is required for enabling the feed export feature. See :ref:`topics-feed-storage-backends` for supported URI schemes. @@ -318,17 +319,43 @@ For instance:: } The following is a list of the accepted keys and the setting that is used -as a fallback value if that key is not provided for a specific feed definition. +as a fallback value if that key is not provided for a specific feed definition: + +- ``format``: the :ref:`serialization format `. + + This setting is mandatory, there is no fallback value. + +- ``batch_item_count``: falls back to + :setting:`FEED_EXPORT_BATCH_ITEM_COUNT`. + +- ``encoding``: falls back to :setting:`FEED_EXPORT_ENCODING`. + +- ``fields``: falls back to :setting:`FEED_EXPORT_FIELDS`. + +- ``indent``: falls back to :setting:`FEED_EXPORT_INDENT`. + +- ``overwrite``: whether to overwrite the file if it already exists + (``True``) or append to its content (``False``). + + The default value depends on the :ref:`storage backend + `: + + - :ref:`topics-feed-storage-fs`: ``False`` + + - :ref:`topics-feed-storage-ftp`: ``True`` + + .. note:: Some FTP servers may not support appending to files (the + ``APPE`` FTP command). + + - :ref:`topics-feed-storage-s3`: ``True`` (appending `is not supported + `_) + + - :ref:`topics-feed-storage-stdout`: ``False`` (overwriting is not supported) + +- ``store_empty``: falls back to :setting:`FEED_STORE_EMPTY`. + +- ``uri_params``: falls back to :setting:`FEED_URI_PARAMS`. -* ``format``: the serialization format to be used for the feed. - See :ref:`topics-feed-format` for possible values. - Mandatory, no fallback setting -* ``batch_item_count``: falls back to :setting:`FEED_EXPORT_BATCH_ITEM_COUNT` -* ``encoding``: falls back to :setting:`FEED_EXPORT_ENCODING` -* ``fields``: falls back to :setting:`FEED_EXPORT_FIELDS` -* ``indent``: falls back to :setting:`FEED_EXPORT_INDENT` -* ``store_empty``: falls back to :setting:`FEED_STORE_EMPTY` -* ``uri_params``: falls back to :setting:`FEED_URI_PARAMS` .. setting:: FEED_EXPORT_ENCODING diff --git a/scrapy/commands/__init__.py b/scrapy/commands/__init__.py index 57ce4e522..cfd940fe7 100644 --- a/scrapy/commands/__init__.py +++ b/scrapy/commands/__init__.py @@ -115,9 +115,11 @@ class BaseRunSpiderCommand(ScrapyCommand): parser.add_option("-a", dest="spargs", action="append", default=[], metavar="NAME=VALUE", help="set spider argument (may be repeated)") parser.add_option("-o", "--output", metavar="FILE", action="append", - help="dump scraped items into FILE (use - for stdout)") + help="append scraped items to the end of FILE (use - for stdout)") + parser.add_option("-O", "--overwrite-output", metavar="FILE", action="append", + help="dump scraped items into FILE, overwriting any existing file") parser.add_option("-t", "--output-format", metavar="FORMAT", - help="format to use for dumping items with -o") + help="format to use for dumping items") def process_options(self, args, opts): ScrapyCommand.process_options(self, args, opts) @@ -125,6 +127,11 @@ class BaseRunSpiderCommand(ScrapyCommand): opts.spargs = arglist_to_dict(opts.spargs) except ValueError: raise UsageError("Invalid -a value, use -a NAME=VALUE", print_help=False) - if opts.output: - feeds = feed_process_params_from_cli(self.settings, opts.output, opts.output_format) + if opts.output or opts.overwrite_output: + feeds = feed_process_params_from_cli( + self.settings, + opts.output, + opts.output_format, + opts.overwrite_output, + ) self.settings.set('FEEDS', feeds, priority='cmdline') diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index b7a4e362e..980825499 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -24,17 +24,34 @@ from scrapy.utils.conf import feed_complete_default_values_from_settings from scrapy.utils.ftp import ftp_store_file from scrapy.utils.log import failure_to_exc_info from scrapy.utils.misc import create_instance, load_object -from scrapy.utils.python import without_none_values +from scrapy.utils.python import get_func_args, without_none_values logger = logging.getLogger(__name__) +def build_storage(builder, uri, *args, feed_options=None, preargs=(), **kwargs): + argument_names = get_func_args(builder) + if 'feed_options' in argument_names: + kwargs['feed_options'] = feed_options + else: + warnings.warn( + "{} does not support the 'feed_options' keyword argument. Add a " + "'feed_options' parameter to its signature to remove this " + "warning. This parameter will become mandatory in a future " + "version of Scrapy." + .format(builder.__qualname__), + category=ScrapyDeprecationWarning + ) + return builder(*preargs, uri, *args, **kwargs) + + class IFeedStorage(Interface): """Interface that all Feed Storages must implement""" - def __init__(uri): - """Initialize the storage with the parameters given in the URI""" + def __init__(uri, *, feed_options=None): + """Initialize the storage with the parameters given in the URI and the + feed-specific options (see :setting:`FEEDS`)""" def open(spider): """Open the storage for the given spider. It must return a file-like @@ -64,10 +81,15 @@ class BlockingFeedStorage: @implementer(IFeedStorage) class StdoutFeedStorage: - def __init__(self, uri, _stdout=None): + def __init__(self, uri, _stdout=None, *, feed_options=None): if not _stdout: _stdout = sys.stdout.buffer self._stdout = _stdout + if feed_options and feed_options.get('overwrite', False) is True: + logger.warning('Standard output (stdout) storage does not support ' + 'overwriting. To suppress this warning, remove the ' + 'overwrite option from your FEEDS setting, or set ' + 'it to False.') def open(self, spider): return self._stdout @@ -79,14 +101,16 @@ class StdoutFeedStorage: @implementer(IFeedStorage) class FileFeedStorage: - def __init__(self, uri): + def __init__(self, uri, *, feed_options=None): self.path = file_uri_to_path(uri) + feed_options = feed_options or {} + self.write_mode = 'wb' if feed_options.get('overwrite', False) else 'ab' def open(self, spider): dirname = os.path.dirname(self.path) if dirname and not os.path.exists(dirname): os.makedirs(dirname) - return open(self.path, 'ab') + return open(self.path, self.write_mode) def store(self, file): file.close() @@ -94,7 +118,8 @@ class FileFeedStorage: class S3FeedStorage(BlockingFeedStorage): - def __init__(self, uri, access_key=None, secret_key=None, acl=None): + def __init__(self, uri, access_key=None, secret_key=None, acl=None, *, + feed_options=None): u = urlparse(uri) self.bucketname = u.hostname self.access_key = u.username or access_key @@ -111,14 +136,20 @@ class S3FeedStorage(BlockingFeedStorage): else: import boto self.connect_s3 = boto.connect_s3 + if feed_options and feed_options.get('overwrite', True) is False: + logger.warning('S3 does not support appending to files. To ' + 'suppress this warning, remove the overwrite ' + 'option from your FEEDS setting or set it to True.') @classmethod - def from_crawler(cls, crawler, uri): - return cls( - uri=uri, + def from_crawler(cls, crawler, uri, *, feed_options=None): + return build_storage( + cls, + uri, access_key=crawler.settings['AWS_ACCESS_KEY_ID'], secret_key=crawler.settings['AWS_SECRET_ACCESS_KEY'], - acl=crawler.settings['FEED_STORAGE_S3_ACL'] or None + acl=crawler.settings['FEED_STORAGE_S3_ACL'] or None, + feed_options=feed_options, ) def _store_in_thread(self, file): @@ -135,6 +166,7 @@ class S3FeedStorage(BlockingFeedStorage): kwargs = {'policy': self.acl} if self.acl else {} key.set_contents_from_file(file, **kwargs) key.close() + file.close() class GCSFeedStorage(BlockingFeedStorage): @@ -165,27 +197,31 @@ class GCSFeedStorage(BlockingFeedStorage): class FTPFeedStorage(BlockingFeedStorage): - def __init__(self, uri, use_active_mode=False): + def __init__(self, uri, use_active_mode=False, *, feed_options=None): u = urlparse(uri) self.host = u.hostname self.port = int(u.port or '21') self.username = u.username - self.password = unquote(u.password) + self.password = unquote(u.password or '') self.path = u.path self.use_active_mode = use_active_mode + self.overwrite = not feed_options or feed_options.get('overwrite', True) @classmethod - def from_crawler(cls, crawler, uri): - return cls( - uri=uri, - use_active_mode=crawler.settings.getbool('FEED_STORAGE_FTP_ACTIVE') + def from_crawler(cls, crawler, uri, *, feed_options=None): + return build_storage( + cls, + uri, + crawler.settings.getbool('FEED_STORAGE_FTP_ACTIVE'), + feed_options=feed_options, ) def _store_in_thread(self, file): ftp_store_file( path=self.path, file=file, host=self.host, port=self.port, username=self.username, - password=self.password, use_active_mode=self.use_active_mode + password=self.password, use_active_mode=self.use_active_mode, + overwrite=self.overwrite, ) @@ -242,32 +278,32 @@ class FeedExporter: category=ScrapyDeprecationWarning, stacklevel=2, ) uri = str(self.settings['FEED_URI']) # handle pathlib.Path objects - feed = {'format': self.settings.get('FEED_FORMAT', 'jsonlines')} - self.feeds[uri] = feed_complete_default_values_from_settings(feed, self.settings) + feed_options = {'format': self.settings.get('FEED_FORMAT', 'jsonlines')} + self.feeds[uri] = feed_complete_default_values_from_settings(feed_options, self.settings) # End: Backward compatibility for FEED_URI and FEED_FORMAT settings # 'FEEDS' setting takes precedence over 'FEED_URI' - for uri, feed in self.settings.getdict('FEEDS').items(): + for uri, feed_options in self.settings.getdict('FEEDS').items(): uri = str(uri) # handle pathlib.Path objects - self.feeds[uri] = feed_complete_default_values_from_settings(feed, self.settings) + self.feeds[uri] = feed_complete_default_values_from_settings(feed_options, self.settings) self.storages = self._load_components('FEED_STORAGES') self.exporters = self._load_components('FEED_EXPORTERS') - for uri, feed in self.feeds.items(): - if not self._storage_supported(uri): + for uri, feed_options in self.feeds.items(): + if not self._storage_supported(uri, feed_options): raise NotConfigured if not self._settings_are_valid(): raise NotConfigured - if not self._exporter_supported(feed['format']): + if not self._exporter_supported(feed_options['format']): raise NotConfigured def open_spider(self, spider): - for uri, feed in self.feeds.items(): - uri_params = self._get_uri_params(spider, feed['uri_params']) + for uri, feed_options in self.feeds.items(): + uri_params = self._get_uri_params(spider, feed_options['uri_params']) self.slots.append(self._start_new_batch( batch_id=1, uri=uri % uri_params, - feed=feed, + feed_options=feed_options, spider=spider, uri_template=uri, )) @@ -306,32 +342,32 @@ class FeedExporter: ) return d - def _start_new_batch(self, batch_id, uri, feed, spider, uri_template): + def _start_new_batch(self, batch_id, uri, feed_options, spider, uri_template): """ Redirect the output data stream to a new file. Execute multiple times if FEED_EXPORT_BATCH_ITEM_COUNT setting or FEEDS.batch_item_count is specified :param batch_id: sequence number of current batch :param uri: uri of the new batch to start - :param feed: dict with parameters of feed + :param feed_options: dict with parameters of feed :param spider: user spider :param uri_template: template of uri which contains %(batch_time)s or %(batch_id)d to create new uri """ - storage = self._get_storage(uri) + storage = self._get_storage(uri, feed_options) file = storage.open(spider) exporter = self._get_exporter( file=file, - format=feed['format'], - fields_to_export=feed['fields'], - encoding=feed['encoding'], - indent=feed['indent'], + format=feed_options['format'], + fields_to_export=feed_options['fields'], + encoding=feed_options['encoding'], + indent=feed_options['indent'], ) slot = _FeedSlot( file=file, exporter=exporter, storage=storage, uri=uri, - format=feed['format'], - store_empty=feed['store_empty'], + format=feed_options['format'], + store_empty=feed_options['store_empty'], batch_id=batch_id, uri_template=uri_template, ) @@ -355,7 +391,7 @@ class FeedExporter: slots.append(self._start_new_batch( batch_id=slot.batch_id + 1, uri=slot.uri_template % uri_params, - feed=self.feeds[slot.uri_template], + feed_options=self.feeds[slot.uri_template], spider=spider, uri_template=slot.uri_template, )) @@ -394,11 +430,11 @@ class FeedExporter: return False return True - def _storage_supported(self, uri): + def _storage_supported(self, uri, feed_options): scheme = urlparse(uri).scheme if scheme in self.storages: try: - self._get_storage(uri) + self._get_storage(uri, feed_options) return True except NotConfigured as e: logger.error("Disabled feed storage scheme: %(scheme)s. " @@ -416,8 +452,30 @@ class FeedExporter: def _get_exporter(self, file, format, *args, **kwargs): return self._get_instance(self.exporters[format], file, *args, **kwargs) - def _get_storage(self, uri): - return self._get_instance(self.storages[urlparse(uri).scheme], uri) + def _get_storage(self, uri, feed_options): + """Fork of create_instance specific to feed storage classes + + It supports not passing the *feed_options* parameters to classes that + do not support it, and issuing a deprecation warning instead. + """ + feedcls = self.storages[urlparse(uri).scheme] + crawler = getattr(self, 'crawler', None) + + def build_instance(builder, *preargs): + return build_storage(builder, uri, preargs=preargs) + + if crawler and hasattr(feedcls, 'from_crawler'): + instance = build_instance(feedcls.from_crawler, crawler) + method_name = 'from_crawler' + elif hasattr(feedcls, 'from_settings'): + instance = build_instance(feedcls.from_settings, self.settings) + method_name = 'from_settings' + else: + instance = build_instance(feedcls) + method_name = '__new__' + if instance is None: + raise TypeError("%s.%s returned None" % (feedcls.__qualname__, method_name)) + return instance def _get_uri_params(self, spider, uri_params, slot=None): params = {} diff --git a/scrapy/utils/conf.py b/scrapy/utils/conf.py index a83076c47..90a52b25b 100644 --- a/scrapy/utils/conf.py +++ b/scrapy/utils/conf.py @@ -127,7 +127,8 @@ def feed_complete_default_values_from_settings(feed, settings): return out -def feed_process_params_from_cli(settings, output, output_format=None): +def feed_process_params_from_cli(settings, output, output_format=None, + overwrite_output=None): """ Receives feed export params (from the 'crawl' or 'runspider' commands), checks for inconsistencies in their quantities and returns a dictionary @@ -139,22 +140,39 @@ def feed_process_params_from_cli(settings, output, output_format=None): def check_valid_format(output_format): if output_format not in valid_output_formats: - raise UsageError("Unrecognized output format '%s', set one after a" - " colon using the -o option (i.e. -o :)" - " or as a file extension, from the supported list %s" % - (output_format, tuple(valid_output_formats))) + raise UsageError( + "Unrecognized output format '%s'. Set a supported one (%s) " + "after a colon at the end of the output URI (i.e. -o/-O " + ":) or as a file extension." % ( + output_format, + tuple(valid_output_formats), + ) + ) + + overwrite = False + if overwrite_output: + if output: + raise UsageError( + "Please use only one of -o/--output and -O/--overwrite-output" + ) + output = overwrite_output + overwrite = True if output_format: if len(output) == 1: check_valid_format(output_format) - warnings.warn('The -t command line option is deprecated in favor' - ' of specifying the output format within the -o' - ' option, please check the -o option docs for more details', - category=ScrapyDeprecationWarning, stacklevel=2) + message = ( + 'The -t command line option is deprecated in favor of ' + 'specifying the output format within the output URI. See the ' + 'documentation of the -o and -O options for more information.', + ) + warnings.warn(message, ScrapyDeprecationWarning, stacklevel=2) return {output[0]: {'format': output_format}} else: - raise UsageError('The -t command line option cannot be used if multiple' - ' output files are specified with the -o option') + raise UsageError( + 'The -t command-line option cannot be used if multiple output ' + 'URIs are specified' + ) result = {} for element in output: @@ -168,8 +186,10 @@ def feed_process_params_from_cli(settings, output, output_format=None): feed_uri = 'stdout:' check_valid_format(feed_format) result[feed_uri] = {'format': feed_format} + if overwrite: + result[feed_uri]['overwrite'] = True - # FEEDS setting should take precedence over the -o and -t CLI options + # FEEDS setting should take precedence over the matching CLI options result.update(settings.getdict('FEEDS')) return result diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index f07bdd748..19d56d6ec 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -20,7 +20,7 @@ def ftp_makedirs_cwd(ftp, path, first_call=True): def ftp_store_file( *, path, file, host, port, - username, password, use_active_mode=False): + username, password, use_active_mode=False, overwrite=True): """Opens a FTP connection with passed credentials,sets current directory to the directory extracted from given path, then uploads the file to server """ @@ -32,4 +32,6 @@ def ftp_store_file( file.seek(0) dirname, filename = posixpath.split(path) ftp_makedirs_cwd(ftp, dirname) - ftp.storbinary('STOR %s' % filename, file) + command = 'STOR' if overwrite else 'APPE' + ftp.storbinary('%s %s' % (command, filename), file) + file.close() diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 59f1b8371..1f2333264 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -198,7 +198,8 @@ def _getargspec_py23(func): def get_func_args(func, stripself=False): """Return the argument name list of a callable""" if inspect.isfunction(func): - func_args, _, _, _ = _getargspec_py23(func) + spec = inspect.getfullargspec(func) + func_args = spec.args + spec.kwonlyargs elif inspect.isclass(func): return get_func_args(func.__init__, True) elif inspect.ismethod(func): diff --git a/tests/ftpserver.py b/tests/ftpserver.py new file mode 100644 index 000000000..6f0289e08 --- /dev/null +++ b/tests/ftpserver.py @@ -0,0 +1,24 @@ +from argparse import ArgumentParser + +from pyftpdlib.authorizers import DummyAuthorizer +from pyftpdlib.handlers import FTPHandler +from pyftpdlib.servers import FTPServer + + +def main(): + parser = ArgumentParser() + parser.add_argument('-d', '--directory') + args = parser.parse_args() + + authorizer = DummyAuthorizer() + full_permissions = 'elradfmwMT' + authorizer.add_anonymous(args.directory, perm=full_permissions) + handler = FTPHandler + handler.authorizer = authorizer + address = ('127.0.0.1', 2121) + server = FTPServer(address, handler) + server.serve_forever() + + +if __name__ == '__main__': + main() diff --git a/tests/mockserver.py b/tests/mockserver.py index 1f40473ba..48d7b8d37 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -3,7 +3,10 @@ import json import os import random import sys +from pathlib import Path +from shutil import rmtree from subprocess import Popen, PIPE +from tempfile import mkdtemp from urllib.parse import urlencode from OpenSSL import SSL @@ -256,6 +259,29 @@ class MockDNSServer: self.proc.communicate() +class MockFTPServer: + """Creates an FTP server on port 2121 with a default passwordless user + (anonymous) and a temporary root path that you can read from the + :attr:`path` attribute.""" + + def __enter__(self): + self.path = Path(mkdtemp()) + self.proc = Popen([sys.executable, '-u', '-m', 'tests.ftpserver', '-d', str(self.path)], + stderr=PIPE, env=get_testenv()) + for line in self.proc.stderr: + if b'starting FTP server' in line: + break + return self + + def __exit__(self, exc_type, exc_value, traceback): + rmtree(str(self.path)) + self.proc.kill() + self.proc.communicate() + + def url(self, path): + return 'ftp://127.0.0.1:2121/' + path + + def ssl_context_factory(keyfile='keys/localhost.key', certfile='keys/localhost.crt', cipher_string=None): factory = ssl.DefaultOpenSSLContextFactory( os.path.join(os.path.dirname(__file__), keyfile), diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index b51177abb..fe1cbc997 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -4,6 +4,7 @@ dataclasses; python_version == '3.6' mitmproxy; python_version >= '3.7' mitmproxy >= 4, < 5; python_version >= '3.6' and python_version < '3.7' mitmproxy < 4; python_version < '3.6' +pyftpdlib # https://github.com/pytest-dev/pytest-twisted/issues/93 pytest != 5.4, != 5.4.1 pytest-azurepipelines diff --git a/tests/test_commands.py b/tests/test_commands.py index 109f006a8..f76f851e7 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -74,6 +74,7 @@ class ProjectTest(unittest.TestCase): def kill_proc(): p.kill() + p.communicate() assert False, 'Command took too much time to complete' timer = Timer(15, kill_proc) @@ -569,6 +570,55 @@ class BadSpider(scrapy.Spider): log = self.get_log(self.debug_log_spider, args=[]) self.assertNotIn("Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor", log) + def test_output(self): + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + def start_requests(self): + self.logger.debug('FEEDS: {}'.format(self.settings.getdict('FEEDS'))) + return [] +""" + args = ['-o', 'example.json'] + log = self.get_log(spider_code, args=args) + self.assertIn("[myspider] DEBUG: FEEDS: {'example.json': {'format': 'json'}}", log) + + def test_overwrite_output(self): + spider_code = """ +import json +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + def start_requests(self): + self.logger.debug( + 'FEEDS: {}'.format( + json.dumps(self.settings.getdict('FEEDS'), sort_keys=True) + ) + ) + return [] +""" + args = ['-O', 'example.json'] + log = self.get_log(spider_code, args=args) + self.assertIn('[myspider] DEBUG: FEEDS: {"example.json": {"format": "json", "overwrite": true}}', log) + + def test_output_and_overwrite_output(self): + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + def start_requests(self): + return [] +""" + args = ['-o', 'example1.json', '-O', 'example2.json'] + log = self.get_log(spider_code, args=args) + self.assertIn("error: Please use only one of -o/--output and -O/--overwrite-output", log) + class BenchCommandTest(CommandTest): @@ -577,3 +627,79 @@ class BenchCommandTest(CommandTest): '-s', 'CLOSESPIDER_TIMEOUT=0.01') self.assertIn('INFO: Crawled', log) self.assertNotIn('Unhandled Error', log) + + +class CrawlCommandTest(CommandTest): + + def crawl(self, code, args=()): + fname = abspath(join(self.proj_mod_path, 'spiders', 'myspider.py')) + with open(fname, 'w') as f: + f.write(code) + return self.proc('crawl', 'myspider', *args) + + def get_log(self, code, args=()): + _, _, stderr = self.crawl(code, args=args) + return stderr + + def test_no_output(self): + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + def start_requests(self): + self.logger.debug('It works!') + return [] +""" + log = self.get_log(spider_code) + self.assertIn("[myspider] DEBUG: It works!", log) + + def test_output(self): + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + def start_requests(self): + self.logger.debug('FEEDS: {}'.format(self.settings.getdict('FEEDS'))) + return [] +""" + args = ['-o', 'example.json'] + log = self.get_log(spider_code, args=args) + self.assertIn("[myspider] DEBUG: FEEDS: {'example.json': {'format': 'json'}}", log) + + def test_overwrite_output(self): + spider_code = """ +import json +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + def start_requests(self): + self.logger.debug( + 'FEEDS: {}'.format( + json.dumps(self.settings.getdict('FEEDS'), sort_keys=True) + ) + ) + return [] +""" + args = ['-O', 'example.json'] + log = self.get_log(spider_code, args=args) + self.assertIn('[myspider] DEBUG: FEEDS: {"example.json": {"format": "json", "overwrite": true}}', log) + + def test_output_and_overwrite_output(self): + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + def start_requests(self): + return [] +""" + args = ['-o', 'example1.json', '-O', 'example2.json'] + log = self.get_log(spider_code, args=args) + self.assertIn("error: Please use only one of -o/--output and -O/--overwrite-output", log) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 2afc25a7a..850485b5e 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -5,6 +5,7 @@ import random import shutil import string import tempfile +import warnings from abc import ABC, abstractmethod from collections import defaultdict from io import BytesIO @@ -25,7 +26,7 @@ from zope.interface.verify import verifyObject import scrapy from scrapy.crawler import CrawlerRunner -from scrapy.exceptions import NotConfigured +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.exporters import CsvItemExporter from scrapy.extensions.feedexport import ( BlockingFeedStorage, @@ -46,7 +47,7 @@ from scrapy.utils.test import ( mock_google_cloud_storage, ) -from tests.mockserver import MockServer +from tests.mockserver import MockFTPServer, MockServer class FileFeedStorageTest(unittest.TestCase): @@ -75,8 +76,28 @@ class FileFeedStorageTest(unittest.TestCase): st = FileFeedStorage(path) verifyObject(IFeedStorage, st) + def _store(self, feed_options=None): + path = os.path.abspath(self.mktemp()) + storage = FileFeedStorage(path, feed_options=feed_options) + spider = scrapy.Spider("default") + file = storage.open(spider) + file.write(b"content") + storage.store(file) + return path + + def test_append(self): + path = self._store() + return self._assert_stores(FileFeedStorage(path), path, b"contentcontent") + + def test_overwrite(self): + path = self._store({"overwrite": True}) + return self._assert_stores( + FileFeedStorage(path, feed_options={"overwrite": True}), + path + ) + @defer.inlineCallbacks - def _assert_stores(self, storage, path): + def _assert_stores(self, storage, path, expected_content=b"content"): spider = scrapy.Spider("default") file = storage.open(spider) file.write(b"content") @@ -84,7 +105,7 @@ class FileFeedStorageTest(unittest.TestCase): self.assertTrue(os.path.exists(path)) try: with open(path, 'rb') as fp: - self.assertEqual(fp.read(), b"content") + self.assertEqual(fp.read(), expected_content) finally: os.unlink(path) @@ -99,49 +120,74 @@ class FTPFeedStorageTest(unittest.TestCase): spider = TestSpider.from_crawler(crawler) return spider - def test_store(self): - uri = os.environ.get('FEEDTEST_FTP_URI') - path = os.environ.get('FEEDTEST_FTP_PATH') - if not (uri and path): - raise unittest.SkipTest("No FTP server available for testing") - st = FTPFeedStorage(uri) - verifyObject(IFeedStorage, st) - return self._assert_stores(st, path) + def _store(self, uri, content, feed_options=None, settings=None): + crawler = get_crawler(settings_dict=settings or {}) + storage = FTPFeedStorage.from_crawler( + crawler, + uri, + feed_options=feed_options, + ) + verifyObject(IFeedStorage, storage) + spider = self.get_test_spider() + file = storage.open(spider) + file.write(content) + return storage.store(file) - def test_store_active_mode(self): - uri = os.environ.get('FEEDTEST_FTP_URI') - path = os.environ.get('FEEDTEST_FTP_PATH') - if not (uri and path): - raise unittest.SkipTest("No FTP server available for testing") - use_active_mode = {'FEED_STORAGE_FTP_ACTIVE': True} - crawler = get_crawler(settings_dict=use_active_mode) - st = FTPFeedStorage.from_crawler(crawler, uri) - verifyObject(IFeedStorage, st) - return self._assert_stores(st, path) + def _assert_stored(self, path, content): + self.assertTrue(path.exists()) + try: + with path.open('rb') as fp: + self.assertEqual(fp.read(), content) + finally: + os.unlink(str(path)) + + @defer.inlineCallbacks + def test_append(self): + with MockFTPServer() as ftp_server: + filename = 'file' + url = ftp_server.url(filename) + feed_options = {'overwrite': False} + yield self._store(url, b"foo", feed_options=feed_options) + yield self._store(url, b"bar", feed_options=feed_options) + self._assert_stored(ftp_server.path / filename, b"foobar") + + @defer.inlineCallbacks + def test_overwrite(self): + with MockFTPServer() as ftp_server: + filename = 'file' + url = ftp_server.url(filename) + yield self._store(url, b"foo") + yield self._store(url, b"bar") + self._assert_stored(ftp_server.path / filename, b"bar") + + @defer.inlineCallbacks + def test_append_active_mode(self): + with MockFTPServer() as ftp_server: + settings = {'FEED_STORAGE_FTP_ACTIVE': True} + filename = 'file' + url = ftp_server.url(filename) + feed_options = {'overwrite': False} + yield self._store(url, b"foo", feed_options=feed_options, settings=settings) + yield self._store(url, b"bar", feed_options=feed_options, settings=settings) + self._assert_stored(ftp_server.path / filename, b"foobar") + + @defer.inlineCallbacks + def test_overwrite_active_mode(self): + with MockFTPServer() as ftp_server: + settings = {'FEED_STORAGE_FTP_ACTIVE': True} + filename = 'file' + url = ftp_server.url(filename) + yield self._store(url, b"foo", settings=settings) + yield self._store(url, b"bar", settings=settings) + self._assert_stored(ftp_server.path / filename, b"bar") def test_uri_auth_quote(self): # RFC3986: 3.2.1. User Information pw_quoted = quote(string.punctuation, safe='') - st = FTPFeedStorage('ftp://foo:%s@example.com/some_path' % pw_quoted) + st = FTPFeedStorage('ftp://foo:%s@example.com/some_path' % pw_quoted, + {}) self.assertEqual(st.password, string.punctuation) - @defer.inlineCallbacks - def _assert_stores(self, storage, path): - spider = self.get_test_spider() - file = storage.open(spider) - file.write(b"content") - yield storage.store(file) - self.assertTrue(os.path.exists(path)) - try: - with open(path, 'rb') as fp: - self.assertEqual(fp.read(), b"content") - # again, to check s3 objects are overwritten - yield storage.store(BytesIO(b"new content")) - with open(path, 'rb') as fp: - self.assertEqual(fp.read(), b"new content") - finally: - os.unlink(path) - class BlockingFeedStorageTest(unittest.TestCase): @@ -190,8 +236,10 @@ class S3FeedStorageTest(unittest.TestCase): 'AWS_SECRET_ACCESS_KEY': 'settings_secret'} crawler = get_crawler(settings_dict=aws_credentials) # Instantiate with crawler - storage = S3FeedStorage.from_crawler(crawler, - 's3://mybucket/export.csv') + storage = S3FeedStorage.from_crawler( + crawler, + 's3://mybucket/export.csv', + ) self.assertEqual(storage.access_key, 'settings_key') self.assertEqual(storage.secret_key, 'settings_secret') # Instantiate directly @@ -254,7 +302,7 @@ class S3FeedStorageTest(unittest.TestCase): crawler = get_crawler(settings_dict=settings) storage = S3FeedStorage.from_crawler( crawler, - 's3://mybucket/export.csv' + 's3://mybucket/export.csv', ) self.assertEqual(storage.access_key, 'access_key') self.assertEqual(storage.secret_key, 'secret_key') @@ -269,7 +317,7 @@ class S3FeedStorageTest(unittest.TestCase): crawler = get_crawler(settings_dict=settings) storage = S3FeedStorage.from_crawler( crawler, - 's3://mybucket/export.csv' + 's3://mybucket/export.csv', ) self.assertEqual(storage.access_key, 'access_key') self.assertEqual(storage.secret_key, 'secret_key') @@ -370,6 +418,27 @@ class S3FeedStorageTest(unittest.TestCase): key.set_contents_from_file.call_args ) + def test_overwrite_default(self): + with LogCapture() as log: + S3FeedStorage( + 's3://mybucket/export.csv', + 'access_key', + 'secret_key', + 'custom-acl' + ) + self.assertNotIn('S3 does not support appending to files', str(log)) + + def test_overwrite_false(self): + with LogCapture() as log: + S3FeedStorage( + 's3://mybucket/export.csv', + 'access_key', + 'secret_key', + 'custom-acl', + feed_options={'overwrite': False}, + ) + self.assertIn('S3 does not support appending to files', str(log)) + class GCSFeedStorageTest(unittest.TestCase): @@ -439,12 +508,22 @@ class StdoutFeedStorageTest(unittest.TestCase): yield storage.store(file) self.assertEqual(out.getvalue(), b"content") + def test_overwrite_default(self): + with LogCapture() as log: + StdoutFeedStorage('stdout:') + self.assertNotIn('Standard output (stdout) storage does not support overwriting', str(log)) + + def test_overwrite_true(self): + with LogCapture() as log: + StdoutFeedStorage('stdout:', feed_options={'overwrite': True}) + self.assertIn('Standard output (stdout) storage does not support overwriting', str(log)) + class FromCrawlerMixin: init_with_crawler = False @classmethod - def from_crawler(cls, crawler, *args, **kwargs): + def from_crawler(cls, crawler, *args, feed_options=None, **kwargs): cls.init_with_crawler = True return cls(*args, **kwargs) @@ -454,7 +533,11 @@ class FromCrawlerCsvItemExporter(CsvItemExporter, FromCrawlerMixin): class FromCrawlerFileFeedStorage(FileFeedStorage, FromCrawlerMixin): - pass + + @classmethod + def from_crawler(cls, crawler, *args, feed_options=None, **kwargs): + cls.init_with_crawler = True + return cls(*args, feed_options=feed_options, **kwargs) class DummyBlockingFeedStorage(BlockingFeedStorage): @@ -588,8 +671,8 @@ class FeedExportTest(FeedExportTestBase): FEEDS = settings.get('FEEDS') or {} settings['FEEDS'] = { - printf_escape(path_to_url(file_path)): feed - for file_path, feed in FEEDS.items() + printf_escape(path_to_url(file_path)): feed_options + for file_path, feed_options in FEEDS.items() } content = {} @@ -599,12 +682,12 @@ class FeedExportTest(FeedExportTestBase): spider_cls.start_urls = [s.url('/')] yield runner.crawl(spider_cls) - for file_path, feed in FEEDS.items(): + for file_path, feed_options in FEEDS.items(): if not os.path.exists(str(file_path)): continue with open(str(file_path), 'rb') as f: - content[feed['format']] = f.read() + content[feed_options['format']] = f.read() finally: for file_path in FEEDS.keys(): @@ -1542,3 +1625,262 @@ class BatchDeliveriesTest(FeedExportTestBase): content = json.loads(content.decode('utf-8')) expected_batch, items = items[:batch_size], items[batch_size:] self.assertEqual(expected_batch, content) + + +class FeedExportInitTest(unittest.TestCase): + + def test_unsupported_storage(self): + settings = { + 'FEEDS': { + 'unsupported://uri': {}, + }, + } + crawler = get_crawler(settings_dict=settings) + with self.assertRaises(NotConfigured): + FeedExporter.from_crawler(crawler) + + def test_unsupported_format(self): + settings = { + 'FEEDS': { + 'file://path': { + 'format': 'unsupported_format', + }, + }, + } + crawler = get_crawler(settings_dict=settings) + with self.assertRaises(NotConfigured): + FeedExporter.from_crawler(crawler) + + +class StdoutFeedStorageWithoutFeedOptions(StdoutFeedStorage): + + def __init__(self, uri): + super().__init__(uri) + + +class StdoutFeedStoragePreFeedOptionsTest(unittest.TestCase): + """Make sure that any feed exporter created by users before the + introduction of the ``feed_options`` parameter continues to work as + expected, and simply issues a warning.""" + + def test_init(self): + settings_dict = { + 'FEED_URI': 'file:///tmp/foobar', + 'FEED_STORAGES': { + 'file': 'tests.test_feedexport.StdoutFeedStorageWithoutFeedOptions' + }, + } + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") + with warnings.catch_warnings(record=True) as w: + feed_exporter.open_spider(spider) + messages = tuple(str(item.message) for item in w + if item.category is ScrapyDeprecationWarning) + self.assertEqual( + messages, + ( + ( + "StdoutFeedStorageWithoutFeedOptions does not support " + "the 'feed_options' keyword argument. Add a " + "'feed_options' parameter to its signature to remove " + "this warning. This parameter will become mandatory " + "in a future version of Scrapy." + ), + ) + ) + + +class FileFeedStorageWithoutFeedOptions(FileFeedStorage): + + def __init__(self, uri): + super().__init__(uri) + + +class FileFeedStoragePreFeedOptionsTest(unittest.TestCase): + """Make sure that any feed exporter created by users before the + introduction of the ``feed_options`` parameter continues to work as + expected, and simply issues a warning.""" + + maxDiff = None + + def test_init(self): + settings_dict = { + 'FEED_URI': 'file:///tmp/foobar', + 'FEED_STORAGES': { + 'file': 'tests.test_feedexport.FileFeedStorageWithoutFeedOptions' + }, + } + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") + with warnings.catch_warnings(record=True) as w: + feed_exporter.open_spider(spider) + messages = tuple(str(item.message) for item in w + if item.category is ScrapyDeprecationWarning) + self.assertEqual( + messages, + ( + ( + "FileFeedStorageWithoutFeedOptions does not support " + "the 'feed_options' keyword argument. Add a " + "'feed_options' parameter to its signature to remove " + "this warning. This parameter will become mandatory " + "in a future version of Scrapy." + ), + ) + ) + + +class S3FeedStorageWithoutFeedOptions(S3FeedStorage): + + def __init__(self, uri, access_key, secret_key, acl): + super().__init__(uri, access_key, secret_key, acl) + + +class S3FeedStorageWithoutFeedOptionsWithFromCrawler(S3FeedStorage): + + @classmethod + def from_crawler(cls, crawler, uri): + return super().from_crawler(crawler, uri) + + +class S3FeedStoragePreFeedOptionsTest(unittest.TestCase): + """Make sure that any feed exporter created by users before the + introduction of the ``feed_options`` parameter continues to work as + expected, and simply issues a warning.""" + + maxDiff = None + + def test_init(self): + settings_dict = { + 'FEED_URI': 'file:///tmp/foobar', + 'FEED_STORAGES': { + 'file': 'tests.test_feedexport.S3FeedStorageWithoutFeedOptions' + }, + } + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") + spider.crawler = crawler + with warnings.catch_warnings(record=True) as w: + feed_exporter.open_spider(spider) + messages = tuple(str(item.message) for item in w + if item.category is ScrapyDeprecationWarning) + self.assertEqual( + messages, + ( + ( + "S3FeedStorageWithoutFeedOptions does not support " + "the 'feed_options' keyword argument. Add a " + "'feed_options' parameter to its signature to remove " + "this warning. This parameter will become mandatory " + "in a future version of Scrapy." + ), + ) + ) + + def test_from_crawler(self): + settings_dict = { + 'FEED_URI': 'file:///tmp/foobar', + 'FEED_STORAGES': { + 'file': 'tests.test_feedexport.S3FeedStorageWithoutFeedOptionsWithFromCrawler' + }, + } + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") + spider.crawler = crawler + with warnings.catch_warnings(record=True) as w: + feed_exporter.open_spider(spider) + messages = tuple(str(item.message) for item in w + if item.category is ScrapyDeprecationWarning) + self.assertEqual( + messages, + ( + ( + "S3FeedStorageWithoutFeedOptionsWithFromCrawler.from_crawler " + "does not support the 'feed_options' keyword argument. Add a " + "'feed_options' parameter to its signature to remove " + "this warning. This parameter will become mandatory " + "in a future version of Scrapy." + ), + ) + ) + + +class FTPFeedStorageWithoutFeedOptions(FTPFeedStorage): + + def __init__(self, uri, use_active_mode=False): + super().__init__(uri) + + +class FTPFeedStorageWithoutFeedOptionsWithFromCrawler(FTPFeedStorage): + + @classmethod + def from_crawler(cls, crawler, uri): + return super().from_crawler(crawler, uri) + + +class FTPFeedStoragePreFeedOptionsTest(unittest.TestCase): + """Make sure that any feed exporter created by users before the + introduction of the ``feed_options`` parameter continues to work as + expected, and simply issues a warning.""" + + maxDiff = None + + def test_init(self): + settings_dict = { + 'FEED_URI': 'file:///tmp/foobar', + 'FEED_STORAGES': { + 'file': 'tests.test_feedexport.FTPFeedStorageWithoutFeedOptions' + }, + } + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") + spider.crawler = crawler + with warnings.catch_warnings(record=True) as w: + feed_exporter.open_spider(spider) + messages = tuple(str(item.message) for item in w + if item.category is ScrapyDeprecationWarning) + self.assertEqual( + messages, + ( + ( + "FTPFeedStorageWithoutFeedOptions does not support " + "the 'feed_options' keyword argument. Add a " + "'feed_options' parameter to its signature to remove " + "this warning. This parameter will become mandatory " + "in a future version of Scrapy." + ), + ) + ) + + def test_from_crawler(self): + settings_dict = { + 'FEED_URI': 'file:///tmp/foobar', + 'FEED_STORAGES': { + 'file': 'tests.test_feedexport.FTPFeedStorageWithoutFeedOptionsWithFromCrawler' + }, + } + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") + spider.crawler = crawler + with warnings.catch_warnings(record=True) as w: + feed_exporter.open_spider(spider) + messages = tuple(str(item.message) for item in w + if item.category is ScrapyDeprecationWarning) + self.assertEqual( + messages, + ( + ( + "FTPFeedStorageWithoutFeedOptionsWithFromCrawler.from_crawler " + "does not support the 'feed_options' keyword argument. Add a " + "'feed_options' parameter to its signature to remove " + "this warning. This parameter will become mandatory " + "in a future version of Scrapy." + ), + ) + ) diff --git a/tests/test_utils_conf.py b/tests/test_utils_conf.py index f3ef36127..ccc65c4fd 100644 --- a/tests/test_utils_conf.py +++ b/tests/test_utils_conf.py @@ -141,6 +141,22 @@ class FeedExportConfigTestCase(unittest.TestCase): feed_process_params_from_cli(settings, ['-:pickle']) ) + def test_feed_export_config_overwrite(self): + settings = Settings() + self.assertEqual( + {'output.json': {'format': 'json', 'overwrite': True}}, + feed_process_params_from_cli(settings, [], None, ['output.json']) + ) + + def test_output_and_overwrite_output(self): + with self.assertRaises(UsageError): + feed_process_params_from_cli( + Settings(), + ['output1.json'], + None, + ['output2.json'], + ) + def test_feed_complete_default_values_from_settings_empty(self): feed = {} settings = Settings({ diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index 3f93f509e..c298d0bd2 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -179,6 +179,9 @@ class UtilsPythonTestCase(unittest.TestCase): def f2(a, b=None, c=None): pass + def f3(a, b=None, *, c=None): + pass + class A: def __init__(self, a, b, c): pass @@ -199,6 +202,7 @@ class UtilsPythonTestCase(unittest.TestCase): self.assertEqual(get_func_args(f1), ['a', 'b', 'c']) self.assertEqual(get_func_args(f2), ['a', 'b', 'c']) + self.assertEqual(get_func_args(f3), ['a', 'b', 'c']) self.assertEqual(get_func_args(A), ['a', 'b', 'c']) self.assertEqual(get_func_args(a.method), ['a', 'b', 'c']) self.assertEqual(get_func_args(partial_f1), ['b', 'c']) From 42383cc267393c3c5fb89c9108759125939ab3e5 Mon Sep 17 00:00:00 2001 From: sakshamb2113 <44064539+sakshamb2113@users.noreply.github.com> Date: Wed, 19 Aug 2020 12:48:14 +0530 Subject: [PATCH 17/22] Add a setting to customize the asyncio event loop (#4414) --- docs/topics/asyncio.rst | 12 +++++++++++ docs/topics/settings.rst | 20 ++++++++++++++++++ scrapy/crawler.py | 2 +- scrapy/settings/default_settings.py | 2 ++ scrapy/utils/log.py | 7 +++++++ scrapy/utils/reactor.py | 12 ++++++++--- tests/CrawlerProcess/asyncio_custom_loop.py | 17 +++++++++++++++ tests/requirements-py3.txt | 1 + tests/test_commands.py | 23 +++++++++++++++++++++ tests/test_crawler.py | 8 +++++++ 10 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 tests/CrawlerProcess/asyncio_custom_loop.py diff --git a/docs/topics/asyncio.rst b/docs/topics/asyncio.rst index 038a459fd..bfb430d52 100644 --- a/docs/topics/asyncio.rst +++ b/docs/topics/asyncio.rst @@ -26,3 +26,15 @@ reactor manually. You can do that using :func:`~scrapy.utils.reactor.install_reactor`:: install_reactor('twisted.internet.asyncioreactor.AsyncioSelectorReactor') + +.. _using-custom-loops: + +Using custom asyncio loops +========================== + +You can also use custom asyncio event loops with the asyncio reactor. Set the +:setting:`ASYNCIO_EVENT_LOOP` setting to the import path of the desired event loop class to +use it instead of the default asyncio event loop. + + + diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 722ae4593..618b9989e 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -216,6 +216,26 @@ Default: ``None`` The name of the region associated with the AWS client. +.. setting:: ASYNCIO_EVENT_LOOP + +ASYNCIO_EVENT_LOOP +------------------ + +Default: ``None`` + +Import path of a given asyncio event loop class. + +If the asyncio reactor is enabled (see :setting:`TWISTED_REACTOR`) this setting can be used to specify the +asyncio event loop to be used with it. Set the setting to the import path of the +desired asyncio event loop class. If the setting is set to ``None`` the default asyncio +event loop will be used. + +If you are installing the asyncio reactor manually using the :func:`~scrapy.utils.reactor.install_reactor` +function, you can use the ``event_loop_path`` parameter to indicate the import path of the event loop +class to be used. + +Note that the event loop class must inherit from :class:`asyncio.AbstractEventLoop`. + .. setting:: BOT_NAME BOT_NAME diff --git a/scrapy/crawler.py b/scrapy/crawler.py index d028bea4d..4c6b0e496 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -340,5 +340,5 @@ class CrawlerProcess(CrawlerRunner): def _handle_twisted_reactor(self): if self.settings.get("TWISTED_REACTOR"): - install_reactor(self.settings["TWISTED_REACTOR"]) + install_reactor(self.settings["TWISTED_REACTOR"], self.settings["ASYNCIO_EVENT_LOOP"]) super()._handle_twisted_reactor() diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 896afa995..a0251394b 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -19,6 +19,8 @@ from os.path import join, abspath, dirname AJAXCRAWL_ENABLED = False +ASYNCIO_EVENT_LOOP = None + AUTOTHROTTLE_ENABLED = False AUTOTHROTTLE_DEBUG = False AUTOTHROTTLE_MAX_DELAY = 60.0 diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 1d6a2c39d..e41315738 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -150,6 +150,13 @@ def log_scrapy_info(settings): logger.info("Versions: %(versions)s", {'versions': ", ".join(versions)}) from twisted.internet import reactor logger.debug("Using reactor: %s.%s", reactor.__module__, reactor.__class__.__name__) + from twisted.internet import asyncioreactor + if isinstance(reactor, asyncioreactor.AsyncioSelectorReactor): + logger.debug( + "Using asyncio event loop: %s.%s", + reactor._asyncioEventloop.__module__, + reactor._asyncioEventloop.__class__.__name__, + ) class StreamLogger: diff --git a/scrapy/utils/reactor.py b/scrapy/utils/reactor.py index 3c705f69b..879d27907 100644 --- a/scrapy/utils/reactor.py +++ b/scrapy/utils/reactor.py @@ -50,13 +50,19 @@ class CallLaterOnce: return self._func(*self._a, **self._kw) -def install_reactor(reactor_path): +def install_reactor(reactor_path, event_loop_path=None): """Installs the :mod:`~twisted.internet.reactor` with the specified - import path.""" + import path. Also installs the asyncio event loop with the specified import + path if the asyncio reactor is enabled""" reactor_class = load_object(reactor_path) if reactor_class is asyncioreactor.AsyncioSelectorReactor: with suppress(error.ReactorAlreadyInstalledError): - asyncioreactor.install(asyncio.get_event_loop()) + if event_loop_path is not None: + event_loop_class = load_object(event_loop_path) + event_loop = event_loop_class() + else: + event_loop = asyncio.new_event_loop() + asyncioreactor.install(eventloop=event_loop) else: *module, _ = reactor_path.split(".") installer_path = module + ["install"] diff --git a/tests/CrawlerProcess/asyncio_custom_loop.py b/tests/CrawlerProcess/asyncio_custom_loop.py new file mode 100644 index 000000000..1e4ada722 --- /dev/null +++ b/tests/CrawlerProcess/asyncio_custom_loop.py @@ -0,0 +1,17 @@ +import scrapy +from scrapy.crawler import CrawlerProcess + + +class NoRequestsSpider(scrapy.Spider): + name = 'no_request' + + def start_requests(self): + return [] + + +process = CrawlerProcess(settings={ + "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", + "ASYNCIO_EVENT_LOOP": "uvloop.Loop" +}) +process.crawl(NoRequestsSpider) +process.start() diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index fe1cbc997..44ddcded8 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -13,6 +13,7 @@ pytest-twisted >= 1.11 pytest-xdist sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 testfixtures +uvloop; platform_system != "Windows" # optional for shell wrapper tests bpython diff --git a/tests/test_commands.py b/tests/test_commands.py index f76f851e7..ee8a92604 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -16,6 +16,7 @@ from tempfile import mkdtemp from threading import Timer from unittest import skipIf +from pytest import mark from twisted.trial import unittest import scrapy @@ -570,6 +571,28 @@ class BadSpider(scrapy.Spider): log = self.get_log(self.debug_log_spider, args=[]) self.assertNotIn("Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor", log) + @mark.skipif(sys.implementation.name == 'pypy', reason='uvloop does not support pypy properly') + @mark.skipif(platform.system() == 'Windows', reason='uvloop does not support Windows') + def test_custom_asyncio_loop_enabled_true(self): + log = self.get_log(self.debug_log_spider, args=[ + '-s', + 'TWISTED_REACTOR=twisted.internet.asyncioreactor.AsyncioSelectorReactor', + '-s', + 'ASYNCIO_EVENT_LOOP=uvloop.Loop', + ]) + self.assertIn("Using asyncio event loop: uvloop.Loop", log) + + # https://twistedmatrix.com/trac/ticket/9766 + @skipIf(platform.system() == 'Windows' and sys.version_info >= (3, 8), + "the asyncio reactor is broken on Windows when running Python ≥ 3.8") + def test_custom_asyncio_loop_enabled_false(self): + log = self.get_log(self.debug_log_spider, args=[ + '-s', 'TWISTED_REACTOR=twisted.internet.asyncioreactor.AsyncioSelectorReactor' + ]) + import asyncio + loop = asyncio.new_event_loop() + self.assertIn("Using asyncio event loop: %s.%s" % (loop.__module__, loop.__class__.__name__), log) + def test_output(self): spider_code = """ import scrapy diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 1a4cfe813..7c2e251a9 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -345,6 +345,14 @@ class CrawlerProcessSubprocess(ScriptRunnerMixin, unittest.TestCase): self.assertIn("Spider closed (finished)", log) self.assertIn("Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor", log) + @mark.skipif(sys.implementation.name == 'pypy', reason='uvloop does not support pypy properly') + @mark.skipif(platform.system() == 'Windows', reason='uvloop does not support Windows') + def test_custom_loop_asyncio(self): + log = self.run_script("asyncio_custom_loop.py") + self.assertIn("Spider closed (finished)", log) + self.assertIn("Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor", log) + self.assertIn("Using asyncio event loop: uvloop.Loop", log) + class CrawlerRunnerSubprocess(ScriptRunnerMixin, unittest.TestCase): script_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'CrawlerRunner') From a57db9e3024fe1426021b8b692a48f4c8db82a77 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Wed, 19 Aug 2020 18:45:24 +0300 Subject: [PATCH 18/22] Bitbucket no longer supports Mercurial repositories (#4738) --- .travis.yml | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index db720b918..33a920bb6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -50,7 +50,7 @@ install: - | if [[ ! -z "$PYPY_VERSION" ]]; then export PYPY_VERSION="pypy$PYPY_VERSION-linux64" - wget "https://bitbucket.org/pypy/pypy/downloads/${PYPY_VERSION}.tar.bz2" + wget "https://downloads.python.org/pypy/${PYPY_VERSION}.tar.bz2" tar -jxf ${PYPY_VERSION}.tar.bz2 virtualenv --python="$PYPY_VERSION/bin/pypy3" "$HOME/virtualenvs/$PYPY_VERSION" source "$HOME/virtualenvs/$PYPY_VERSION/bin/activate" diff --git a/setup.py b/setup.py index d0880051f..52a27c368 100644 --- a/setup.py +++ b/setup.py @@ -41,7 +41,7 @@ if has_environment_marker_platform_impl_support(): ] extras_require[':platform_python_implementation == "PyPy"'] = [ # Earlier lxml versions are affected by - # https://bitbucket.org/pypy/pypy/issues/2498/cython-on-pypy-3-dict-object-has-no, + # https://foss.heptapod.net/pypy/pypy/-/issues/2498, # which was fixed in Cython 0.26, released on 2017-06-19, and used to # generate the C headers of lxml release tarballs published since then, the # first of which was: From d68aab992e435aa1c88061e83d03e57e7d31533d Mon Sep 17 00:00:00 2001 From: Grisha Temchenko Date: Thu, 20 Aug 2020 09:22:07 -0400 Subject: [PATCH 19/22] Smarter generator check for combined return/yield statements (#4721) --- scrapy/utils/misc.py | 19 ++++++++++++++++++- ...t_return_with_argument_inside_generator.py | 19 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index d6966be8e..bd400bd30 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -5,6 +5,7 @@ import os import re import hashlib import warnings +from collections import deque from contextlib import contextmanager from importlib import import_module from pkgutil import iter_modules @@ -184,6 +185,22 @@ def set_environ(**kwargs): os.environ[k] = v +def walk_callable(node): + """Similar to ``ast.walk``, but walks only function body and skips nested + functions defined within the node. + """ + todo = deque([node]) + walked_func_def = False + while todo: + node = todo.popleft() + if isinstance(node, ast.FunctionDef): + if walked_func_def: + continue + walked_func_def = True + todo.extend(ast.iter_child_nodes(node)) + yield node + + _generator_callbacks_cache = LocalWeakReferencedCache(limit=128) @@ -201,7 +218,7 @@ def is_generator_with_return_value(callable): if inspect.isgeneratorfunction(callable): tree = ast.parse(dedent(inspect.getsource(callable))) - for node in ast.walk(tree): + for node in walk_callable(tree): if isinstance(node, ast.Return) and not returns_none(node): _generator_callbacks_cache[callable] = True return _generator_callbacks_cache[callable] diff --git a/tests/test_utils_misc/test_return_with_argument_inside_generator.py b/tests/test_utils_misc/test_return_with_argument_inside_generator.py index bdbec1beb..2be38620c 100644 --- a/tests/test_utils_misc/test_return_with_argument_inside_generator.py +++ b/tests/test_utils_misc/test_return_with_argument_inside_generator.py @@ -29,9 +29,28 @@ class UtilsMiscPy3TestCase(unittest.TestCase): yield 1 yield from g() + def m(): + yield 1 + + def helper(): + return 0 + + yield helper() + + def n(): + yield 1 + + def helper(): + return 0 + + yield helper() + return 2 + assert is_generator_with_return_value(f) assert is_generator_with_return_value(g) assert not is_generator_with_return_value(h) assert not is_generator_with_return_value(i) assert not is_generator_with_return_value(j) assert not is_generator_with_return_value(k) # not recursive + assert not is_generator_with_return_value(m) + assert is_generator_with_return_value(n) From 2fbfe2c21411480c35a99ff5497ee59f2e5e0dbb Mon Sep 17 00:00:00 2001 From: yogendra0sharma Date: Fri, 21 Aug 2020 12:18:15 +0530 Subject: [PATCH 20/22] Removed appveyor.xml no longer needed --- appveyor.yml | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 appveyor.yml diff --git a/appveyor.yml b/appveyor.yml deleted file mode 100644 index 7fd636864..000000000 --- a/appveyor.yml +++ /dev/null @@ -1,25 +0,0 @@ -platform: x86 -version: '{branch}-{build}' -environment: - matrix: - - PYTHON: "C:\\Python36" - TOX_ENV: py36 - -branches: - only: - - master - - /d+\.\d+\.\d+[\w\-]*$/ - -install: - - "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%" - - "SET PYTHONPATH=%APPVEYOR_BUILD_FOLDER%" - - "SET TOX_TESTENV_PASSENV=HOME HOMEDRIVE HOMEPATH PYTHONPATH USERPROFILE" - - "pip install -U tox" - -build: false -skip_tags: true -test_script: - - "tox -e %TOX_ENV%" - -cache: - - '%LOCALAPPDATA%\pip\cache' From 7c076122ebb16a6db9b85b68c85c354c2a49fd2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 21 Aug 2020 17:06:54 +0200 Subject: [PATCH 21/22] Skip checks introduced in Pylint 2.6.0 --- pylintrc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pylintrc b/pylintrc index 129c7bf7d..5b6b9fab0 100644 --- a/pylintrc +++ b/pylintrc @@ -68,6 +68,7 @@ disable=abstract-method, pointless-statement, pointless-string-statement, protected-access, + raise-missing-from, redefined-argument-from-local, redefined-builtin, redefined-outer-name, @@ -75,6 +76,7 @@ disable=abstract-method, signature-differs, singleton-comparison, super-init-not-called, + super-with-arguments, superfluous-parens, too-few-public-methods, too-many-ancestors, From f1250177dc1c486517b9ca8ed136162533014da7 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <1731933+elacuesta@users.noreply.github.com> Date: Sat, 22 Aug 2020 04:33:35 -0300 Subject: [PATCH 22/22] Remove Python 3.5 from CI (#4743) --- .travis.yml | 12 +++--------- azure-pipelines.yml | 4 +--- tests/test_utils_python.py | 18 +++++++++--------- tox.ini | 4 ++-- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/.travis.yml b/.travis.yml index 33a920bb6..b883c5b78 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,16 +19,10 @@ matrix: python: 3.8 - env: TOXENV=pinned - python: 3.5.2 + python: 3.6.1 - env: TOXENV=asyncio-pinned - python: 3.5.2 # We use additional code to support 3.5.3 and earlier - - env: TOXENV=pypy3-pinned PYPY_VERSION=3-v5.9.0 - - - env: TOXENV=py - python: 3.5 - - env: TOXENV=asyncio - python: 3.5 # We use specific code to support >= 3.5.4, < 3.6 - - env: TOXENV=pypy3 PYPY_VERSION=3.5-v7.0.0 + python: 3.6.1 + - env: TOXENV=pypy3-pinned PYPY_VERSION=3.6-v7.2.0 - env: TOXENV=py python: 3.6 diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 710e42090..c03e258c7 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -4,11 +4,9 @@ pool: vmImage: 'windows-latest' strategy: matrix: - Python35: - python.version: '3.5' - TOXENV: windows-pinned Python36: python.version: '3.6' + TOXENV: windows-pinned Python37: python.version: '3.7' Python38: diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index c298d0bd2..3115cc92f 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -3,8 +3,8 @@ import gc import operator import platform import unittest +from datetime import datetime from itertools import count -from sys import version_info from warnings import catch_warnings from scrapy.utils.python import ( @@ -216,15 +216,15 @@ class UtilsPythonTestCase(unittest.TestCase): self.assertEqual(get_func_args(str.split), []) self.assertEqual(get_func_args(" ".join), []) self.assertEqual(get_func_args(operator.itemgetter(2)), []) - else: - self.assertEqual( - get_func_args(str.split, stripself=True), ['sep', 'maxsplit']) - self.assertEqual( - get_func_args(operator.itemgetter(2), stripself=True), ['obj']) - if version_info < (3, 6): - self.assertEqual(get_func_args(" ".join, stripself=True), ['list']) - else: + elif platform.python_implementation() == 'PyPy': + self.assertEqual(get_func_args(str.split, stripself=True), ['sep', 'maxsplit']) + self.assertEqual(get_func_args(operator.itemgetter(2), stripself=True), ['obj']) + + build_date = datetime.strptime(platform.python_build()[1], '%b %d %Y') + if build_date >= datetime(2020, 4, 7): # PyPy 3.6-v7.3.1 self.assertEqual(get_func_args(" ".join, stripself=True), ['iterable']) + else: + self.assertEqual(get_func_args(" ".join, stripself=True), ['list']) def test_without_none_values(self): self.assertEqual(without_none_values([1, None, 3, 4]), [1, 3, 4]) diff --git a/tox.ini b/tox.ini index 4f5531aea..dec0d75e8 100644 --- a/tox.ini +++ b/tox.ini @@ -14,7 +14,7 @@ deps = # Extras boto3>=1.13.0 botocore>=1.4.87 - Pillow>=3.4.2 + Pillow>=4.0.0 passenv = S3_TEST_FILE_URI AWS_ACCESS_KEY_ID @@ -78,7 +78,7 @@ deps = # Extras botocore==1.4.87 google-cloud-storage==1.29.0 - Pillow==3.4.2 + Pillow==4.0.0 [testenv:pinned] deps =