From 3d77f74e4089c1a9700fb9e2bc62fc196176250c Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Tue, 5 Nov 2019 00:54:46 -0300 Subject: [PATCH 01/14] Download handlers: from_crawler factory method, take crawler instead of settings in __init__ --- scrapy/core/downloader/handlers/__init__.py | 8 ++- scrapy/core/downloader/handlers/datauri.py | 3 -- scrapy/core/downloader/handlers/file.py | 4 +- scrapy/core/downloader/handlers/ftp.py | 13 +++-- scrapy/core/downloader/handlers/http10.py | 20 +++++-- scrapy/core/downloader/handlers/http11.py | 12 +++-- scrapy/core/downloader/handlers/s3.py | 15 +++--- tests/test_downloader_handlers.py | 58 +++++++++++++-------- 8 files changed, 86 insertions(+), 47 deletions(-) diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index 0b55d32fa..e8beb2f5a 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -5,7 +5,7 @@ from twisted.internet import defer import six from scrapy.exceptions import NotSupported, NotConfigured from scrapy.utils.httpobj import urlparse_cached -from scrapy.utils.misc import load_object +from scrapy.utils.misc import create_instance, load_object from scrapy.utils.python import without_none_values from scrapy import signals @@ -48,7 +48,11 @@ class DownloadHandlers(object): dhcls = load_object(path) if skip_lazy and getattr(dhcls, 'lazy', True): return None - dh = dhcls(self._crawler.settings) + dh = create_instance( + dhcls, + self._crawler.settings, + self._crawler, + ) except NotConfigured as ex: self._notconfigured[scheme] = str(ex) return None diff --git a/scrapy/core/downloader/handlers/datauri.py b/scrapy/core/downloader/handlers/datauri.py index 9e5020753..97134e618 100644 --- a/scrapy/core/downloader/handlers/datauri.py +++ b/scrapy/core/downloader/handlers/datauri.py @@ -8,9 +8,6 @@ from scrapy.utils.decorators import defers class DataURIDownloadHandler(object): lazy = False - def __init__(self, settings): - super(DataURIDownloadHandler, self).__init__() - @defers def download_request(self, request, spider): uri = parse_data_uri(request.url) diff --git a/scrapy/core/downloader/handlers/file.py b/scrapy/core/downloader/handlers/file.py index 23f25d28d..d445ba2e1 100644 --- a/scrapy/core/downloader/handlers/file.py +++ b/scrapy/core/downloader/handlers/file.py @@ -1,4 +1,5 @@ from w3lib.url import file_uri_to_path + from scrapy.responsetypes import responsetypes from scrapy.utils.decorators import defers @@ -6,9 +7,6 @@ from scrapy.utils.decorators import defers class FileDownloadHandler(object): lazy = False - def __init__(self, settings): - pass - @defers def download_request(self, request, spider): filepath = file_uri_to_path(request.url) diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 39ed67a1a..7a98361ed 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -59,6 +59,7 @@ class ReceivedDataProtocol(Protocol): def close(self): self.body.close() if self.filename else self.body.seek(0) + _CODE_RE = re.compile(r"\d+") @@ -70,10 +71,14 @@ class FTPDownloadHandler(object): "default": 503, } - def __init__(self, settings): - self.default_user = settings['FTP_USER'] - self.default_password = settings['FTP_PASSWORD'] - self.passive_mode = settings['FTP_PASSIVE_MODE'] + def __init__(self, crawler): + self.default_user = crawler.settings['FTP_USER'] + self.default_password = crawler.settings['FTP_PASSWORD'] + self.passive_mode = crawler.settings['FTP_PASSIVE_MODE'] + + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) def download_request(self, request, spider): parsed_url = urlparse_cached(request) diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index be7298531..ce0801bcc 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -1,6 +1,7 @@ """Download handlers for http and https schemes """ from twisted.internet import reactor + from scrapy.utils.misc import load_object, create_instance from scrapy.utils.python import to_unicode @@ -8,10 +9,15 @@ from scrapy.utils.python import to_unicode class HTTP10DownloadHandler(object): lazy = False - def __init__(self, settings): - self.HTTPClientFactory = load_object(settings['DOWNLOADER_HTTPCLIENTFACTORY']) - self.ClientContextFactory = load_object(settings['DOWNLOADER_CLIENTCONTEXTFACTORY']) - self._settings = settings + def __init__(self, crawler): + self.HTTPClientFactory = load_object(crawler.settings['DOWNLOADER_HTTPCLIENTFACTORY']) + self.ClientContextFactory = load_object(crawler.settings['DOWNLOADER_CLIENTCONTEXTFACTORY']) + self._crawler = crawler + self._settings = crawler.settings + + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) def download_request(self, request, spider): """Return a deferred for the HTTP download""" @@ -22,7 +28,11 @@ class HTTP10DownloadHandler(object): def _connect(self, factory): host, port = to_unicode(factory.host), factory.port if factory.scheme == b'https': - client_context_factory = create_instance(self.ClientContextFactory, settings=self._settings, crawler=None) + client_context_factory = create_instance( + self.ClientContextFactory, + settings=self._settings, + crawler=self._crawler, + ) return reactor.connectSSL(host, port, factory, client_context_factory) else: return reactor.connectTCP(host, port, factory) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 7d917cb74..b424a7999 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -30,7 +30,9 @@ logger = logging.getLogger(__name__) class HTTP11DownloadHandler(object): lazy = False - def __init__(self, settings): + def __init__(self, crawler): + settings = crawler.settings + self._pool = HTTPConnectionPool(reactor, persistent=True) self._pool.maxPersistentPerHost = settings.getint('CONCURRENT_REQUESTS_PER_DOMAIN') self._pool._factory.noisy = False @@ -42,7 +44,7 @@ class HTTP11DownloadHandler(object): self._contextFactory = create_instance( self._contextFactoryClass, settings=settings, - crawler=None, + crawler=crawler, method=self._sslMethod, ) except TypeError: @@ -50,7 +52,7 @@ class HTTP11DownloadHandler(object): self._contextFactory = create_instance( self._contextFactoryClass, settings=settings, - crawler=None, + crawler=crawler, ) msg = """ '%s' does not accept `method` argument (type OpenSSL.SSL method,\ @@ -63,6 +65,10 @@ class HTTP11DownloadHandler(object): self._fail_on_dataloss = settings.getbool('DOWNLOAD_FAIL_ON_DATALOSS') self._disconnect_timeout = 1 + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + def download_request(self, request, spider): """Return a deferred for the HTTP download""" agent = ScrapyAgent( diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 808d1bf21..220296fb3 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -32,13 +32,12 @@ def _get_boto_connection(): class S3DownloadHandler(object): - def __init__(self, settings, aws_access_key_id=None, aws_secret_access_key=None, \ - httpdownloadhandler=HTTPDownloadHandler, **kw): - + def __init__(self, crawler, aws_access_key_id=None, aws_secret_access_key=None, + httpdownloadhandler=HTTPDownloadHandler, **kw): if not aws_access_key_id: - aws_access_key_id = settings['AWS_ACCESS_KEY_ID'] + aws_access_key_id = crawler.settings['AWS_ACCESS_KEY_ID'] if not aws_secret_access_key: - aws_secret_access_key = settings['AWS_SECRET_ACCESS_KEY'] + aws_secret_access_key = crawler.settings['AWS_SECRET_ACCESS_KEY'] # If no credentials could be found anywhere, # consider this an anonymous connection request by default; @@ -67,7 +66,11 @@ class S3DownloadHandler(object): except Exception as ex: raise NotConfigured(str(ex)) - self._download_http = httpdownloadhandler(settings).download_request + self._download_http = httpdownloadhandler(crawler).download_request + + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) def download_request(self, request, spider): p = urlparse_cached(request) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 60124b93f..815342219 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -30,7 +30,6 @@ from scrapy.spiders import Spider from scrapy.http import Headers, Request from scrapy.http.response.text import TextResponse from scrapy.responsetypes import responsetypes -from scrapy.settings import Settings from scrapy.utils.test import get_crawler, skip_if_no_boto from scrapy.utils.python import to_bytes from scrapy.exceptions import NotConfigured @@ -45,6 +44,10 @@ class DummyDH(object): def __init__(self, crawler): pass + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + class DummyLazyDH(object): # Default is lazy for backward compatibility @@ -52,6 +55,10 @@ class DummyLazyDH(object): def __init__(self, crawler): pass + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + class OffDH(object): lazy = False @@ -59,6 +66,10 @@ class OffDH(object): def __init__(self, crawler): raise NotConfigured + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + class LoadTestCase(unittest.TestCase): @@ -106,7 +117,7 @@ class FileTestCase(unittest.TestCase): self.tmpname = self.mktemp() with open(self.tmpname + '^', 'w') as f: f.write('0123456789') - self.download_request = FileDownloadHandler(Settings()).download_request + self.download_request = FileDownloadHandler().download_request def tearDown(self): os.unlink(self.tmpname + '^') @@ -239,7 +250,7 @@ class HttpTestCase(unittest.TestCase): else: self.port = reactor.listenTCP(0, self.wrapper, interface=self.host) self.portno = self.port.getHost().port - self.download_handler = self.download_handler_cls(Settings()) + self.download_handler = self.download_handler_cls(get_crawler()) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -479,9 +490,9 @@ class Http11TestCase(HttpTestCase): return self.test_download_broken_content_allow_data_loss('broken-chunked') def test_download_broken_content_allow_data_loss_via_setting(self, url='broken'): - download_handler = self.download_handler_cls(Settings({ - 'DOWNLOAD_FAIL_ON_DATALOSS': False, - })) + download_handler = self.download_handler_cls( + get_crawler(settings_dict={'DOWNLOAD_FAIL_ON_DATALOSS': False}) + ) request = Request(self.getURL(url)) d = download_handler.download_request(request, Spider('foo')) d.addCallback(lambda r: r.flags) @@ -499,9 +510,9 @@ class Https11TestCase(Http11TestCase): @defer.inlineCallbacks def test_tls_logging(self): - download_handler = self.download_handler_cls(Settings({ - 'DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING': True, - })) + download_handler = self.download_handler_cls( + get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING': True}) + ) try: with LogCapture() as log_capture: request = Request(self.getURL('file')) @@ -569,7 +580,8 @@ class Https11CustomCiphers(unittest.TestCase): interface=self.host) self.portno = self.port.getHost().port self.download_handler = self.download_handler_cls( - Settings({'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'})) + get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'}) + ) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -665,7 +677,7 @@ class HttpProxyTestCase(unittest.TestCase): wrapper = WrappingFactory(site) self.port = reactor.listenTCP(0, wrapper, interface='127.0.0.1') self.portno = self.port.getHost().port - self.download_handler = self.download_handler_cls(Settings()) + self.download_handler = self.download_handler_cls(get_crawler()) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -738,9 +750,10 @@ class S3AnonTestCase(unittest.TestCase): def setUp(self): skip_if_no_boto() - self.s3reqh = S3DownloadHandler(Settings(), - httpdownloadhandler=HttpDownloadHandlerMock, - #anon=True, # is implicit + self.s3reqh = S3DownloadHandler( + crawler=get_crawler(), + httpdownloadhandler=HttpDownloadHandlerMock, + #anon=True, # is implicit ) self.download_request = self.s3reqh.download_request self.spider = Spider('foo') @@ -766,9 +779,12 @@ class S3TestCase(unittest.TestCase): def setUp(self): skip_if_no_boto() - s3reqh = S3DownloadHandler(Settings(), self.AWS_ACCESS_KEY_ID, - self.AWS_SECRET_ACCESS_KEY, - httpdownloadhandler=HttpDownloadHandlerMock) + s3reqh = S3DownloadHandler( + get_crawler(), + self.AWS_ACCESS_KEY_ID, + self.AWS_SECRET_ACCESS_KEY, + httpdownloadhandler=HttpDownloadHandlerMock, + ) self.download_request = s3reqh.download_request self.spider = Spider('foo') @@ -788,7 +804,7 @@ class S3TestCase(unittest.TestCase): def test_extra_kw(self): try: - S3DownloadHandler(Settings(), extra_kw=True) + S3DownloadHandler(get_crawler(), extra_kw=True) except Exception as e: self.assertIsInstance(e, (TypeError, NotConfigured)) else: @@ -928,7 +944,7 @@ class BaseFTPTestCase(unittest.TestCase): self.factory = FTPFactory(portal=p) self.port = reactor.listenTCP(0, self.factory, interface="127.0.0.1") self.portNum = self.port.getHost().port - self.download_handler = FTPDownloadHandler(Settings()) + self.download_handler = FTPDownloadHandler(get_crawler()) self.addCleanup(self.port.stopListening) def tearDown(self): @@ -1042,7 +1058,7 @@ class AnonymousFTPTestCase(BaseFTPTestCase): userAnonymous=self.username) self.port = reactor.listenTCP(0, self.factory, interface="127.0.0.1") self.portNum = self.port.getHost().port - self.download_handler = FTPDownloadHandler(Settings()) + self.download_handler = FTPDownloadHandler(get_crawler()) self.addCleanup(self.port.stopListening) def tearDown(self): @@ -1052,7 +1068,7 @@ class AnonymousFTPTestCase(BaseFTPTestCase): class DataURITestCase(unittest.TestCase): def setUp(self): - self.download_handler = DataURIDownloadHandler(Settings()) + self.download_handler = DataURIDownloadHandler() self.download_request = self.download_handler.download_request self.spider = Spider('foo') From e43f37fff3cbdb5c4de7af21594c6062859a50e0 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Tue, 5 Nov 2019 16:18:42 -0300 Subject: [PATCH 02/14] Pass args/kwargs in S3DownloadHandler.from_crawler, update tests --- scrapy/core/downloader/handlers/s3.py | 4 ++-- tests/test_downloader_handlers.py | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 220296fb3..99a3a7925 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -69,8 +69,8 @@ class S3DownloadHandler(object): self._download_http = httpdownloadhandler(crawler).download_request @classmethod - def from_crawler(cls, crawler): - return cls(crawler) + def from_crawler(cls, crawler, *args, **kwargs): + return cls(crawler, *args, **kwargs) def download_request(self, request, spider): p = urlparse_cached(request) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 815342219..eff5653f0 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -250,7 +250,7 @@ class HttpTestCase(unittest.TestCase): else: self.port = reactor.listenTCP(0, self.wrapper, interface=self.host) self.portno = self.port.getHost().port - self.download_handler = self.download_handler_cls(get_crawler()) + self.download_handler = self.download_handler_cls.from_crawler(get_crawler()) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -490,7 +490,7 @@ class Http11TestCase(HttpTestCase): return self.test_download_broken_content_allow_data_loss('broken-chunked') def test_download_broken_content_allow_data_loss_via_setting(self, url='broken'): - download_handler = self.download_handler_cls( + download_handler = self.download_handler_cls.from_crawler( get_crawler(settings_dict={'DOWNLOAD_FAIL_ON_DATALOSS': False}) ) request = Request(self.getURL(url)) @@ -510,7 +510,7 @@ class Https11TestCase(Http11TestCase): @defer.inlineCallbacks def test_tls_logging(self): - download_handler = self.download_handler_cls( + download_handler = self.download_handler_cls.from_crawler( get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING': True}) ) try: @@ -579,7 +579,7 @@ class Https11CustomCiphers(unittest.TestCase): 0, self.wrapper, ssl_context_factory(self.keyfile, self.certfile, cipher_string='CAMELLIA256-SHA'), interface=self.host) self.portno = self.port.getHost().port - self.download_handler = self.download_handler_cls( + self.download_handler = self.download_handler_cls.from_crawler( get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'}) ) self.download_request = self.download_handler.download_request @@ -677,7 +677,7 @@ class HttpProxyTestCase(unittest.TestCase): wrapper = WrappingFactory(site) self.port = reactor.listenTCP(0, wrapper, interface='127.0.0.1') self.portno = self.port.getHost().port - self.download_handler = self.download_handler_cls(get_crawler()) + self.download_handler = self.download_handler_cls.from_crawler(get_crawler()) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -750,7 +750,7 @@ class S3AnonTestCase(unittest.TestCase): def setUp(self): skip_if_no_boto() - self.s3reqh = S3DownloadHandler( + self.s3reqh = S3DownloadHandler.from_crawler( crawler=get_crawler(), httpdownloadhandler=HttpDownloadHandlerMock, #anon=True, # is implicit @@ -779,10 +779,10 @@ class S3TestCase(unittest.TestCase): def setUp(self): skip_if_no_boto() - s3reqh = S3DownloadHandler( - get_crawler(), - self.AWS_ACCESS_KEY_ID, - self.AWS_SECRET_ACCESS_KEY, + s3reqh = S3DownloadHandler.from_crawler( + crawler=get_crawler(), + aws_access_key_id=self.AWS_ACCESS_KEY_ID, + aws_secret_access_key=self.AWS_SECRET_ACCESS_KEY, httpdownloadhandler=HttpDownloadHandlerMock, ) self.download_request = s3reqh.download_request @@ -804,7 +804,7 @@ class S3TestCase(unittest.TestCase): def test_extra_kw(self): try: - S3DownloadHandler(get_crawler(), extra_kw=True) + S3DownloadHandler.from_crawler(get_crawler(), extra_kw=True) except Exception as e: self.assertIsInstance(e, (TypeError, NotConfigured)) else: From 931b7e68d33e06b624b49a7abeababb4e3540474 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 09:50:28 -0300 Subject: [PATCH 03/14] Update FileDownloadHandler test --- scrapy/core/downloader/handlers/file.py | 2 +- tests/test_downloader_handlers.py | 26 +++++++++++++------------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/scrapy/core/downloader/handlers/file.py b/scrapy/core/downloader/handlers/file.py index d445ba2e1..0d94e3df0 100644 --- a/scrapy/core/downloader/handlers/file.py +++ b/scrapy/core/downloader/handlers/file.py @@ -4,7 +4,7 @@ from scrapy.responsetypes import responsetypes from scrapy.utils.decorators import defers -class FileDownloadHandler(object): +class FileDownloadHandler: lazy = False @defers diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 4505f2bf7..ce4685eed 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -1,21 +1,20 @@ +import contextlib import os import shutil import tempfile from unittest import mock -import contextlib from testfixtures import LogCapture -from twisted.trial import unittest +from twisted.cred import checkers, credentials, portal +from twisted.internet import defer, error, reactor from twisted.protocols.policies import WrappingFactory from twisted.python.filepath import FilePath -from twisted.internet import reactor, defer, error -from twisted.web import server, static, util, resource +from twisted.trial import unittest +from twisted.web import resource, server, static, util from twisted.web._newclient import ResponseFailed from twisted.web.http import _DataLoss -from twisted.web.test.test_webclient import ForeverTakingResource, \ - NoLengthResource, HostHeaderResource, \ - PayloadResource -from twisted.cred import portal, checkers, credentials +from twisted.web.test.test_webclient import (ForeverTakingResource, HostHeaderResource, + NoLengthResource, PayloadResource) from w3lib.url import path_to_file_uri from scrapy.core.downloader.handlers import DownloadHandlers @@ -26,13 +25,14 @@ from scrapy.core.downloader.handlers.http10 import HTTP10DownloadHandler from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler from scrapy.core.downloader.handlers.s3 import S3DownloadHandler -from scrapy.spiders import Spider +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.http import Headers, Request from scrapy.http.response.text import TextResponse from scrapy.responsetypes import responsetypes -from scrapy.utils.test import get_crawler, skip_if_no_boto +from scrapy.spiders import Spider +from scrapy.utils.misc import create_instance from scrapy.utils.python import to_bytes -from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning +from scrapy.utils.test import get_crawler, skip_if_no_boto from tests.mockserver import MockServer, ssl_context_factory, Echo from tests.spiders import SingleRequestSpider @@ -117,7 +117,9 @@ class FileTestCase(unittest.TestCase): self.tmpname = self.mktemp() with open(self.tmpname + '^', 'w') as f: f.write('0123456789') - self.download_request = FileDownloadHandler().download_request + crawler = get_crawler() + handler = create_instance(FileDownloadHandler, crawler.settings, crawler) + self.download_request = handler.download_request def tearDown(self): os.unlink(self.tmpname + '^') From 342bf3cd35df856f4f2eafbf2717e9244c9deaf8 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 09:52:55 -0300 Subject: [PATCH 04/14] Explicit keyword arguments --- scrapy/core/downloader/handlers/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index e8c4454d2..94e0e59ef 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -50,9 +50,9 @@ class DownloadHandlers(object): if skip_lazy and getattr(dhcls, 'lazy', True): return None dh = create_instance( - dhcls, - self._crawler.settings, - self._crawler, + objcls=dhcls, + settings=self._crawler.settings, + crawler=self._crawler, ) except NotConfigured as ex: self._notconfigured[scheme] = str(ex) From 9e5d945ef27ff9efee54b2245e833fefc3df72d8 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 09:55:47 -0300 Subject: [PATCH 05/14] Use create_instance in downloader handler tests --- tests/test_downloader_handlers.py | 35 ++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index ce4685eed..b63e8405e 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -252,7 +252,12 @@ class HttpTestCase(unittest.TestCase): else: self.port = reactor.listenTCP(0, self.wrapper, interface=self.host) self.portno = self.port.getHost().port - self.download_handler = self.download_handler_cls.from_crawler(get_crawler()) + crawler = get_crawler() + self.download_handler = create_instance( + objcls=self.download_handler_cls, + settings=crawler.settings, + crawler=crawler + ) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -492,8 +497,11 @@ class Http11TestCase(HttpTestCase): return self.test_download_broken_content_allow_data_loss('broken-chunked') def test_download_broken_content_allow_data_loss_via_setting(self, url='broken'): - download_handler = self.download_handler_cls.from_crawler( - get_crawler(settings_dict={'DOWNLOAD_FAIL_ON_DATALOSS': False}) + crawler = get_crawler(settings_dict={'DOWNLOAD_FAIL_ON_DATALOSS': False}) + download_handler = create_instance( + objcls=self.download_handler_cls, + settings=crawler.settings, + crawler=crawler ) request = Request(self.getURL(url)) d = download_handler.download_request(request, Spider('foo')) @@ -512,8 +520,11 @@ class Https11TestCase(Http11TestCase): @defer.inlineCallbacks def test_tls_logging(self): - download_handler = self.download_handler_cls.from_crawler( - get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING': True}) + crawler = get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING': True}) + download_handler = create_instance( + objcls=self.download_handler_cls, + settings=crawler.settings, + crawler=crawler ) try: with LogCapture() as log_capture: @@ -581,8 +592,11 @@ class Https11CustomCiphers(unittest.TestCase): 0, self.wrapper, ssl_context_factory(self.keyfile, self.certfile, cipher_string='CAMELLIA256-SHA'), interface=self.host) self.portno = self.port.getHost().port - self.download_handler = self.download_handler_cls.from_crawler( - get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'}) + crawler = get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'}) + self.download_handler = create_instance( + objcls=self.download_handler_cls, + settings=crawler.settings, + crawler=crawler ) self.download_request = self.download_handler.download_request @@ -679,7 +693,12 @@ class HttpProxyTestCase(unittest.TestCase): wrapper = WrappingFactory(site) self.port = reactor.listenTCP(0, wrapper, interface='127.0.0.1') self.portno = self.port.getHost().port - self.download_handler = self.download_handler_cls.from_crawler(get_crawler()) + crawler = get_crawler() + self.download_handler = create_instance( + objcls=self.download_handler_cls, + settings=crawler.settings, + crawler=crawler + ) self.download_request = self.download_handler.download_request @defer.inlineCallbacks From fa21d8687a0f94e5d79cb28af11312ad58629954 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 10:00:25 -0300 Subject: [PATCH 06/14] Use create_instance in S3DownloadHandler tests --- tests/test_downloader_handlers.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index b63e8405e..ac0e94364 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -776,10 +776,13 @@ class S3AnonTestCase(unittest.TestCase): def setUp(self): skip_if_no_boto() - self.s3reqh = S3DownloadHandler.from_crawler( - crawler=get_crawler(), + crawler = get_crawler() + self.s3reqh = create_instance( + objcls=S3DownloadHandler, + settings=crawler.settings, + crawler=crawler, httpdownloadhandler=HttpDownloadHandlerMock, - #anon=True, # is implicit + # anon=True, # implicit ) self.download_request = self.s3reqh.download_request self.spider = Spider('foo') @@ -805,8 +808,11 @@ class S3TestCase(unittest.TestCase): def setUp(self): skip_if_no_boto() - s3reqh = S3DownloadHandler.from_crawler( - crawler=get_crawler(), + crawler = get_crawler() + s3reqh = create_instance( + objcls=S3DownloadHandler, + settings=crawler.settings, + crawler=crawler, aws_access_key_id=self.AWS_ACCESS_KEY_ID, aws_secret_access_key=self.AWS_SECRET_ACCESS_KEY, httpdownloadhandler=HttpDownloadHandlerMock, @@ -830,7 +836,13 @@ class S3TestCase(unittest.TestCase): def test_extra_kw(self): try: - S3DownloadHandler.from_crawler(get_crawler(), extra_kw=True) + crawler = get_crawler() + create_instance( + objcls=S3DownloadHandler, + settings=crawler.settings, + crawler=crawler, + extra_kw=True, + ) except Exception as e: self.assertIsInstance(e, (TypeError, NotConfigured)) else: From 7e6387de407297a36dd0e9b8e2058ae809cf3123 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 10:02:58 -0300 Subject: [PATCH 07/14] Use create_instance in FTPDownloadHandler/DataURIDownloadHandler tests --- tests/test_downloader_handlers.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index ac0e94364..14d58b651 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -984,7 +984,8 @@ class BaseFTPTestCase(unittest.TestCase): self.factory = FTPFactory(portal=p) self.port = reactor.listenTCP(0, self.factory, interface="127.0.0.1") self.portNum = self.port.getHost().port - self.download_handler = FTPDownloadHandler(get_crawler()) + crawler = get_crawler() + self.download_handler = create_instance(FTPDownloadHandler, crawler.settings, crawler) self.addCleanup(self.port.stopListening) def tearDown(self): @@ -1098,7 +1099,8 @@ class AnonymousFTPTestCase(BaseFTPTestCase): userAnonymous=self.username) self.port = reactor.listenTCP(0, self.factory, interface="127.0.0.1") self.portNum = self.port.getHost().port - self.download_handler = FTPDownloadHandler(get_crawler()) + crawler = get_crawler() + self.download_handler = create_instance(FTPDownloadHandler, crawler.settings, crawler) self.addCleanup(self.port.stopListening) def tearDown(self): @@ -1108,7 +1110,8 @@ class AnonymousFTPTestCase(BaseFTPTestCase): class DataURITestCase(unittest.TestCase): def setUp(self): - self.download_handler = DataURIDownloadHandler() + crawler = get_crawler() + self.download_handler = create_instance(DataURIDownloadHandler, crawler.settings, crawler) self.download_request = self.download_handler.download_request self.spider = Spider('foo') From 8a567e98bbb1c7f917462d0376061303baef5883 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 10:05:49 -0300 Subject: [PATCH 08/14] Remove unnecessary __init__ methods in downloader handler tests --- tests/test_downloader_handlers.py | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 14d58b651..b66b8151e 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -38,29 +38,16 @@ from tests.mockserver import MockServer, ssl_context_factory, Echo from tests.spiders import SingleRequestSpider -class DummyDH(object): +class DummyDH: lazy = False - def __init__(self, crawler): - pass - @classmethod - def from_crawler(cls, crawler): - return cls(crawler) - - -class DummyLazyDH(object): +class DummyLazyDH: # Default is lazy for backward compatibility - - def __init__(self, crawler): - pass - - @classmethod - def from_crawler(cls, crawler): - return cls(crawler) + pass -class OffDH(object): +class OffDH: lazy = False def __init__(self, crawler): @@ -765,7 +752,7 @@ class Http11ProxyTestCase(HttpProxyTestCase): class HttpDownloadHandlerMock(object): - def __init__(self, settings): + def __init__(self, settings, crawler): pass def download_request(self, request, spider): From a6ec89251eca6977f898e8341634bbe7288fd966 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 10:40:16 -0300 Subject: [PATCH 09/14] Downloader handlers: crawler=None in __init__ --- scrapy/core/downloader/handlers/ftp.py | 12 ++++++------ scrapy/core/downloader/handlers/http10.py | 8 ++++---- scrapy/core/downloader/handlers/http11.py | 8 +++----- scrapy/core/downloader/handlers/s3.py | 15 +++++++++------ tests/test_downloader_handlers.py | 4 +--- 5 files changed, 23 insertions(+), 24 deletions(-) diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index fafecc1a8..2b22465e0 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -63,7 +63,7 @@ class ReceivedDataProtocol(Protocol): _CODE_RE = re.compile(r"\d+") -class FTPDownloadHandler(object): +class FTPDownloadHandler: lazy = False CODE_MAPPING = { @@ -71,14 +71,14 @@ class FTPDownloadHandler(object): "default": 503, } - def __init__(self, crawler): - self.default_user = crawler.settings['FTP_USER'] - self.default_password = crawler.settings['FTP_PASSWORD'] - self.passive_mode = crawler.settings['FTP_PASSIVE_MODE'] + def __init__(self, settings, crawler=None): + self.default_user = settings['FTP_USER'] + self.default_password = settings['FTP_PASSWORD'] + self.passive_mode = settings['FTP_PASSIVE_MODE'] @classmethod def from_crawler(cls, crawler): - return cls(crawler) + return cls(crawler.settings, crawler) def download_request(self, request, spider): parsed_url = urlparse_cached(request) diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index ce0801bcc..51c0acd1b 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -6,18 +6,18 @@ from scrapy.utils.misc import load_object, create_instance from scrapy.utils.python import to_unicode -class HTTP10DownloadHandler(object): +class HTTP10DownloadHandler: lazy = False - def __init__(self, crawler): + def __init__(self, settings, crawler=None): self.HTTPClientFactory = load_object(crawler.settings['DOWNLOADER_HTTPCLIENTFACTORY']) self.ClientContextFactory = load_object(crawler.settings['DOWNLOADER_CLIENTCONTEXTFACTORY']) + self._settings = settings self._crawler = crawler - self._settings = crawler.settings @classmethod def from_crawler(cls, crawler): - return cls(crawler) + return cls(crawler.settings, crawler) def download_request(self, request, spider): """Return a deferred for the HTTP download""" diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 691937e97..25dc287df 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -28,12 +28,10 @@ from scrapy.utils.python import to_bytes, to_unicode logger = logging.getLogger(__name__) -class HTTP11DownloadHandler(object): +class HTTP11DownloadHandler: lazy = False - def __init__(self, crawler): - settings = crawler.settings - + def __init__(self, settings, crawler=None): self._pool = HTTPConnectionPool(reactor, persistent=True) self._pool.maxPersistentPerHost = settings.getint('CONCURRENT_REQUESTS_PER_DOMAIN') self._pool._factory.noisy = False @@ -68,7 +66,7 @@ class HTTP11DownloadHandler(object): @classmethod def from_crawler(cls, crawler): - return cls(crawler) + return cls(crawler.settings, crawler) def download_request(self, request, spider): """Return a deferred for the HTTP download""" diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index b35b59f3a..93cad0662 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -4,6 +4,7 @@ from scrapy.core.downloader.handlers.http import HTTPDownloadHandler from scrapy.exceptions import NotConfigured from scrapy.utils.boto import is_botocore from scrapy.utils.httpobj import urlparse_cached +from scrapy.utils.misc import create_instance def _get_boto_connection(): @@ -30,14 +31,15 @@ def _get_boto_connection(): return _S3Connection -class S3DownloadHandler(object): +class S3DownloadHandler: - def __init__(self, crawler, aws_access_key_id=None, aws_secret_access_key=None, + def __init__(self, settings, crawler=None, + aws_access_key_id=None, aws_secret_access_key=None, httpdownloadhandler=HTTPDownloadHandler, **kw): if not aws_access_key_id: - aws_access_key_id = crawler.settings['AWS_ACCESS_KEY_ID'] + aws_access_key_id = settings['AWS_ACCESS_KEY_ID'] if not aws_secret_access_key: - aws_secret_access_key = crawler.settings['AWS_SECRET_ACCESS_KEY'] + aws_secret_access_key = settings['AWS_SECRET_ACCESS_KEY'] # If no credentials could be found anywhere, # consider this an anonymous connection request by default; @@ -66,11 +68,12 @@ class S3DownloadHandler(object): except Exception as ex: raise NotConfigured(str(ex)) - self._download_http = httpdownloadhandler(crawler).download_request + _http_handler = create_instance(httpdownloadhandler, settings, crawler) + self._download_http = _http_handler.download_request @classmethod def from_crawler(cls, crawler, *args, **kwargs): - return cls(crawler, *args, **kwargs) + return cls(crawler.settings, crawler, *args, **kwargs) def download_request(self, request, spider): p = urlparse_cached(request) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index b66b8151e..218360709 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -751,9 +751,7 @@ class Http11ProxyTestCase(HttpProxyTestCase): self.assertIn(domain, timeout.osError) -class HttpDownloadHandlerMock(object): - def __init__(self, settings, crawler): - pass +class HttpDownloadHandlerMock: def download_request(self, request, spider): return request From e2e15d66510c3040234ef9222ff2a17961c50eff Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 10:48:19 -0300 Subject: [PATCH 10/14] Downloader handlers: sort imports --- scrapy/core/downloader/handlers/__init__.py | 6 +++--- scrapy/core/downloader/handlers/datauri.py | 2 +- scrapy/core/downloader/handlers/ftp.py | 4 ++-- scrapy/core/downloader/handlers/http10.py | 2 +- scrapy/core/downloader/handlers/http11.py | 20 ++++++++++---------- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index 94e0e59ef..e86680978 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -4,17 +4,17 @@ import logging from twisted.internet import defer -from scrapy.exceptions import NotSupported, NotConfigured +from scrapy import signals +from scrapy.exceptions import NotConfigured, NotSupported from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.misc import create_instance, load_object from scrapy.utils.python import without_none_values -from scrapy import signals logger = logging.getLogger(__name__) -class DownloadHandlers(object): +class DownloadHandlers: def __init__(self, crawler): self._crawler = crawler diff --git a/scrapy/core/downloader/handlers/datauri.py b/scrapy/core/downloader/handlers/datauri.py index 97134e618..a45b4ff3c 100644 --- a/scrapy/core/downloader/handlers/datauri.py +++ b/scrapy/core/downloader/handlers/datauri.py @@ -5,7 +5,7 @@ from scrapy.responsetypes import responsetypes from scrapy.utils.decorators import defers -class DataURIDownloadHandler(object): +class DataURIDownloadHandler: lazy = False @defers diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 2b22465e0..89c88ad70 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -33,8 +33,8 @@ from io import BytesIO from urllib.parse import unquote from twisted.internet import reactor -from twisted.protocols.ftp import FTPClient, CommandFailed -from twisted.internet.protocol import Protocol, ClientCreator +from twisted.internet.protocol import ClientCreator, Protocol +from twisted.protocols.ftp import CommandFailed, FTPClient from scrapy.http import Response from scrapy.responsetypes import responsetypes diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index 51c0acd1b..1086a6cc0 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -2,7 +2,7 @@ """ from twisted.internet import reactor -from scrapy.utils.misc import load_object, create_instance +from scrapy.utils.misc import create_instance, load_object from scrapy.utils.python import to_unicode diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 25dc287df..d8a561792 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -1,27 +1,27 @@ """Download handlers for http and https schemes""" -import re import logging +import re import warnings from io import BytesIO from time import time from urllib.parse import urldefrag -from zope.interface import implementer -from twisted.internet import defer, reactor, protocol +from twisted.internet import defer, protocol, reactor +from twisted.internet.endpoints import TCP4ClientEndpoint +from twisted.internet.error import TimeoutError +from twisted.web.client import Agent, HTTPConnectionPool, ResponseDone, ResponseFailed, URI +from twisted.web.http import _DataLoss, PotentialDataLoss from twisted.web.http_headers import Headers as TxHeaders from twisted.web.iweb import IBodyProducer, UNKNOWN_LENGTH -from twisted.internet.error import TimeoutError -from twisted.web.http import _DataLoss, PotentialDataLoss -from twisted.web.client import Agent, ResponseDone, HTTPConnectionPool, ResponseFailed, URI -from twisted.internet.endpoints import TCP4ClientEndpoint +from zope.interface import implementer +from scrapy.core.downloader.tls import openssl_methods +from scrapy.core.downloader.webclient import _parse from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Headers from scrapy.responsetypes import responsetypes -from scrapy.core.downloader.webclient import _parse -from scrapy.core.downloader.tls import openssl_methods -from scrapy.utils.misc import load_object, create_instance +from scrapy.utils.misc import create_instance, load_object from scrapy.utils.python import to_bytes, to_unicode From 2fb160e3bac9e09373a49cb0b2d764ddf782987c Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 20:24:16 -0300 Subject: [PATCH 11/14] Use settings instead of crawler --- scrapy/core/downloader/handlers/ftp.py | 4 ++-- scrapy/core/downloader/handlers/http10.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 89c88ad70..1681c6df8 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -71,14 +71,14 @@ class FTPDownloadHandler: "default": 503, } - def __init__(self, settings, crawler=None): + def __init__(self, settings): self.default_user = settings['FTP_USER'] self.default_password = settings['FTP_PASSWORD'] self.passive_mode = settings['FTP_PASSIVE_MODE'] @classmethod def from_crawler(cls, crawler): - return cls(crawler.settings, crawler) + return cls(crawler.settings) def download_request(self, request, spider): parsed_url = urlparse_cached(request) diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index 1086a6cc0..87a42f1da 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -10,8 +10,8 @@ class HTTP10DownloadHandler: lazy = False def __init__(self, settings, crawler=None): - self.HTTPClientFactory = load_object(crawler.settings['DOWNLOADER_HTTPCLIENTFACTORY']) - self.ClientContextFactory = load_object(crawler.settings['DOWNLOADER_CLIENTCONTEXTFACTORY']) + self.HTTPClientFactory = load_object(settings['DOWNLOADER_HTTPCLIENTFACTORY']) + self.ClientContextFactory = load_object(settings['DOWNLOADER_CLIENTCONTEXTFACTORY']) self._settings = settings self._crawler = crawler From 9a75b46fb8322d27ffd45ee6c187bc84a565e26d Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 20:26:58 -0300 Subject: [PATCH 12/14] Explicit argument names --- scrapy/core/downloader/handlers/http10.py | 2 +- scrapy/core/downloader/handlers/http11.py | 4 ++-- scrapy/core/downloader/handlers/s3.py | 6 +++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index 87a42f1da..d4aa51bd1 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -29,7 +29,7 @@ class HTTP10DownloadHandler: host, port = to_unicode(factory.host), factory.port if factory.scheme == b'https': client_context_factory = create_instance( - self.ClientContextFactory, + objcls=self.ClientContextFactory, settings=self._settings, crawler=self._crawler, ) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index d8a561792..5a5f6cf0a 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -41,7 +41,7 @@ class HTTP11DownloadHandler: # try method-aware context factory try: self._contextFactory = create_instance( - self._contextFactoryClass, + objcls=self._contextFactoryClass, settings=settings, crawler=crawler, method=self._sslMethod, @@ -49,7 +49,7 @@ class HTTP11DownloadHandler: except TypeError: # use context factory defaults self._contextFactory = create_instance( - self._contextFactoryClass, + objcls=self._contextFactoryClass, settings=settings, crawler=crawler, ) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 93cad0662..b38d2bf86 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -68,7 +68,11 @@ class S3DownloadHandler: except Exception as ex: raise NotConfigured(str(ex)) - _http_handler = create_instance(httpdownloadhandler, settings, crawler) + _http_handler = create_instance( + objcls=httpdownloadhandler, + settings=settings, + crawler=crawler, + ) self._download_http = _http_handler.download_request @classmethod From 982a66f9fb627575d42f0ad4fb2eb38b0a55b784 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 20:28:17 -0300 Subject: [PATCH 13/14] [test] Download handler: avoid passing settings if not necessary --- tests/test_downloader_handlers.py | 41 +++++++------------------------ 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 218360709..8d95d7cac 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -104,8 +104,7 @@ class FileTestCase(unittest.TestCase): self.tmpname = self.mktemp() with open(self.tmpname + '^', 'w') as f: f.write('0123456789') - crawler = get_crawler() - handler = create_instance(FileDownloadHandler, crawler.settings, crawler) + handler = create_instance(FileDownloadHandler, None, get_crawler()) self.download_request = handler.download_request def tearDown(self): @@ -239,12 +238,7 @@ class HttpTestCase(unittest.TestCase): else: self.port = reactor.listenTCP(0, self.wrapper, interface=self.host) self.portno = self.port.getHost().port - crawler = get_crawler() - self.download_handler = create_instance( - objcls=self.download_handler_cls, - settings=crawler.settings, - crawler=crawler - ) + self.download_handler = create_instance(self.download_handler_cls, None, get_crawler()) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -485,11 +479,7 @@ class Http11TestCase(HttpTestCase): def test_download_broken_content_allow_data_loss_via_setting(self, url='broken'): crawler = get_crawler(settings_dict={'DOWNLOAD_FAIL_ON_DATALOSS': False}) - download_handler = create_instance( - objcls=self.download_handler_cls, - settings=crawler.settings, - crawler=crawler - ) + download_handler = create_instance(self.download_handler_cls, None, crawler) request = Request(self.getURL(url)) d = download_handler.download_request(request, Spider('foo')) d.addCallback(lambda r: r.flags) @@ -508,11 +498,7 @@ class Https11TestCase(Http11TestCase): @defer.inlineCallbacks def test_tls_logging(self): crawler = get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING': True}) - download_handler = create_instance( - objcls=self.download_handler_cls, - settings=crawler.settings, - crawler=crawler - ) + download_handler = create_instance(self.download_handler_cls, None, crawler) try: with LogCapture() as log_capture: request = Request(self.getURL('file')) @@ -580,11 +566,7 @@ class Https11CustomCiphers(unittest.TestCase): interface=self.host) self.portno = self.port.getHost().port crawler = get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'}) - self.download_handler = create_instance( - objcls=self.download_handler_cls, - settings=crawler.settings, - crawler=crawler - ) + self.download_handler = create_instance(self.download_handler_cls, None, crawler) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -680,12 +662,7 @@ class HttpProxyTestCase(unittest.TestCase): wrapper = WrappingFactory(site) self.port = reactor.listenTCP(0, wrapper, interface='127.0.0.1') self.portno = self.port.getHost().port - crawler = get_crawler() - self.download_handler = create_instance( - objcls=self.download_handler_cls, - settings=crawler.settings, - crawler=crawler - ) + self.download_handler = create_instance(self.download_handler_cls, None, get_crawler()) self.download_request = self.download_handler.download_request @defer.inlineCallbacks @@ -764,7 +741,7 @@ class S3AnonTestCase(unittest.TestCase): crawler = get_crawler() self.s3reqh = create_instance( objcls=S3DownloadHandler, - settings=crawler.settings, + settings=None, crawler=crawler, httpdownloadhandler=HttpDownloadHandlerMock, # anon=True, # implicit @@ -796,7 +773,7 @@ class S3TestCase(unittest.TestCase): crawler = get_crawler() s3reqh = create_instance( objcls=S3DownloadHandler, - settings=crawler.settings, + settings=None, crawler=crawler, aws_access_key_id=self.AWS_ACCESS_KEY_ID, aws_secret_access_key=self.AWS_SECRET_ACCESS_KEY, @@ -824,7 +801,7 @@ class S3TestCase(unittest.TestCase): crawler = get_crawler() create_instance( objcls=S3DownloadHandler, - settings=crawler.settings, + settings=None, crawler=crawler, extra_kw=True, ) From ab54e0d33e2a461059a9f1e9759c95b18e38da28 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 20:37:18 -0300 Subject: [PATCH 14/14] Keyword-only args for S3DownloadHandler --- scrapy/core/downloader/handlers/s3.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index b38d2bf86..40a1fa48e 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -33,7 +33,8 @@ def _get_boto_connection(): class S3DownloadHandler: - def __init__(self, settings, crawler=None, + def __init__(self, settings, *, + crawler=None, aws_access_key_id=None, aws_secret_access_key=None, httpdownloadhandler=HTTPDownloadHandler, **kw): if not aws_access_key_id: @@ -76,8 +77,8 @@ class S3DownloadHandler: self._download_http = _http_handler.download_request @classmethod - def from_crawler(cls, crawler, *args, **kwargs): - return cls(crawler.settings, crawler, *args, **kwargs) + def from_crawler(cls, crawler, **kwargs): + return cls(crawler.settings, crawler=crawler, **kwargs) def download_request(self, request, spider): p = urlparse_cached(request)