From c025003da2c60a0a786c2ee158fe620402914273 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Sun, 18 Aug 2019 04:44:09 +0200 Subject: [PATCH 01/81] Add FTPFileStore --- scrapy/pipelines/files.py | 50 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 2145e6d2b..6f66460b8 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -10,6 +10,7 @@ import os.path import time import logging from email.utils import parsedate_tz, mktime_tz +from ftplib import FTP from six.moves.urllib.parse import urlparse from collections import defaultdict import six @@ -31,6 +32,7 @@ from scrapy.utils.python import to_bytes from scrapy.utils.request import referer_str from scrapy.utils.boto import is_botocore from scrapy.utils.datatypes import CaselessDict +from scrapy.utils.ftp import ftp_makedirs_cwd logger = logging.getLogger(__name__) @@ -248,6 +250,42 @@ class GCSFilesStore(object): ) +class FTPFilesStore(object): + + def __init__(self, uri): + assert uri.startswith('ftp://') + u = urlparse(uri) + self.ftp = FTP() + self.ftp.connect(u.hostname, u.port or '21') + self.ftp.login(u.username, u.password) + self.basedir = u.path + '/' + ftp_makedirs_cwd(self.ftp, self.basedir+'full') + + def persist_file(self, path, buf, info, meta=None, headers=None): + buf.seek(0) + filename = path.split('/')[1] + return threads.deferToThread( + self.ftp.storbinary, + 'STOR %s' % filename, + buf + ) + + def stat_file(self, path, info): + def _stat_file(path): + try: + last_modified = float(self.ftp.voidcmd("MDTM " + self.basedir + '/' + path)[4:].strip()) + m = hashlib.md5() + self.ftp.retrbinary('RETR %s' % self.basedir + path, m.update) + return {'last_modified': last_modified, 'checksum': m.hexdigest()} + # The file doesn't exist + except Exception as e : + return {} + return threads.deferToThread(_stat_file, path) + + def close_connection(self): + self.ftp.quit() + + class FilesPipeline(MediaPipeline): """Abstract pipeline that implement the file downloading @@ -274,6 +312,7 @@ class FilesPipeline(MediaPipeline): 'file': FSFilesStore, 's3': S3FilesStore, 'gs': GCSFilesStore, + 'ftp': FTPFilesStore } DEFAULT_FILES_URLS_FIELD = 'file_urls' DEFAULT_FILES_RESULT_FIELD = 'files' @@ -284,7 +323,6 @@ class FilesPipeline(MediaPipeline): if isinstance(settings, dict) or settings is None: settings = Settings(settings) - cls_name = "FilesPipeline" self.store = self._get_store(store_uri) resolve = functools.partial(self._key_for_pipe, @@ -303,7 +341,6 @@ class FilesPipeline(MediaPipeline): self.files_result_field = settings.get( resolve('FILES_RESULT_FIELD'), self.FILES_RESULT_FIELD ) - super(FilesPipeline, self).__init__(download_func=download_func, settings=settings) @classmethod @@ -320,7 +357,7 @@ class FilesPipeline(MediaPipeline): gcs_store = cls.STORE_SCHEMES['gs'] gcs_store.GCS_PROJECT_ID = settings['GCS_PROJECT_ID'] gcs_store.POLICY = settings['FILES_STORE_GCS_ACL'] or None - + store_uri = settings['FILES_STORE'] return cls(store_uri, settings=settings) @@ -461,3 +498,10 @@ class FilesPipeline(MediaPipeline): media_guid = hashlib.sha1(to_bytes(request.url)).hexdigest() media_ext = os.path.splitext(request.url)[1] return 'full/%s%s' % (media_guid, media_ext) + + def close_spider(self, spider): + try: + self.store.close_connection() + # If the store doesn't implement this function, pass + except AttributeError: + pass From 9b1587ed1bc736f9fcc357ce425405adf5bd6d08 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Mon, 19 Aug 2019 16:13:56 +0200 Subject: [PATCH 02/81] Credentials from settings-Support custom paths-Remove close conenction --- scrapy/pipelines/files.py | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 6f66460b8..c22404799 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -251,19 +251,30 @@ class GCSFilesStore(object): class FTPFilesStore(object): + + FTP_USERNAME = None + FTP_PASSWORD = None def __init__(self, uri): assert uri.startswith('ftp://') u = urlparse(uri) self.ftp = FTP() self.ftp.connect(u.hostname, u.port or '21') - self.ftp.login(u.username, u.password) - self.basedir = u.path + '/' - ftp_makedirs_cwd(self.ftp, self.basedir+'full') + username = u.username or FTP_USERNAME + password = u.password or FTP_PASSWORD + self.ftp.login(username, password) + self.basedir = u.path def persist_file(self, path, buf, info, meta=None, headers=None): buf.seek(0) - filename = path.split('/')[1] + # If the path is like 'x/y/z.ext' the 'x/y' is rel_path and + # 'z.ext' is file name + # If path is only the file name 'z.ext', then rel_path is + # the empty string and filename is 'z.ext' + x = path.rsplit('/',1) + rel_path, filename = ('/' + x[0], x[1]) if len(x) > 1 else ('', x[0]) + abs_path = self.basedir + rel_path + ftp_makedirs_cwd(self.ftp, abs_path) return threads.deferToThread( self.ftp.storbinary, 'STOR %s' % filename, @@ -281,9 +292,6 @@ class FTPFilesStore(object): except Exception as e : return {} return threads.deferToThread(_stat_file, path) - - def close_connection(self): - self.ftp.quit() class FilesPipeline(MediaPipeline): @@ -357,6 +365,10 @@ class FilesPipeline(MediaPipeline): gcs_store = cls.STORE_SCHEMES['gs'] gcs_store.GCS_PROJECT_ID = settings['GCS_PROJECT_ID'] gcs_store.POLICY = settings['FILES_STORE_GCS_ACL'] or None + + ftp_store = cls.STORE_SCHEMES['ftp'] + ftp_store.FTP_USERNAME = settings['FTP_USER'] # Default is 'anonymous' + ftp_store.FTP_PASSWORD = settings['FTP_PASSWORD'] # Default is `guest` store_uri = settings['FILES_STORE'] return cls(store_uri, settings=settings) @@ -497,11 +509,4 @@ class FilesPipeline(MediaPipeline): def file_path(self, request, response=None, info=None): media_guid = hashlib.sha1(to_bytes(request.url)).hexdigest() media_ext = os.path.splitext(request.url)[1] - return 'full/%s%s' % (media_guid, media_ext) - - def close_spider(self, spider): - try: - self.store.close_connection() - # If the store doesn't implement this function, pass - except AttributeError: - pass + return 'full/%s%s' % (media_guid, media_ext) \ No newline at end of file From 0a5cb7745bc22b8e193c4b0e964b20e59ddc0bc2 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Mon, 19 Aug 2019 17:12:11 +0200 Subject: [PATCH 03/81] Fix reference mistake --- scrapy/pipelines/files.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index c22404799..cbe588f9a 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -260,8 +260,8 @@ class FTPFilesStore(object): u = urlparse(uri) self.ftp = FTP() self.ftp.connect(u.hostname, u.port or '21') - username = u.username or FTP_USERNAME - password = u.password or FTP_PASSWORD + username = u.username or self.FTP_USERNAME + password = u.password or self.FTP_PASSWORD self.ftp.login(username, password) self.basedir = u.path From 81ac1da3813c11a806aca845a5022e9964b03f80 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Mon, 19 Aug 2019 17:17:21 +0200 Subject: [PATCH 04/81] Handle leading and trailing slashes --- scrapy/pipelines/files.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index cbe588f9a..74697fc1d 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -263,7 +263,7 @@ class FTPFilesStore(object): username = u.username or self.FTP_USERNAME password = u.password or self.FTP_PASSWORD self.ftp.login(username, password) - self.basedir = u.path + self.basedir = u.path.rstrip('/') def persist_file(self, path, buf, info, meta=None, headers=None): buf.seek(0) @@ -272,7 +272,7 @@ class FTPFilesStore(object): # If path is only the file name 'z.ext', then rel_path is # the empty string and filename is 'z.ext' x = path.rsplit('/',1) - rel_path, filename = ('/' + x[0], x[1]) if len(x) > 1 else ('', x[0]) + rel_path, filename = ('/' + x[0].lstrip('/'), x[1]) if len(x) > 1 else ('', x[0]) abs_path = self.basedir + rel_path ftp_makedirs_cwd(self.ftp, abs_path) return threads.deferToThread( From 790bf9031229261639a5c457f6dd3498bdff8830 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Mon, 19 Aug 2019 19:16:47 +0200 Subject: [PATCH 05/81] Make FTP persiting files thread safe --- scrapy/pipelines/files.py | 43 +++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 74697fc1d..2959179b8 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -257,29 +257,32 @@ class FTPFilesStore(object): def __init__(self, uri): assert uri.startswith('ftp://') - u = urlparse(uri) - self.ftp = FTP() - self.ftp.connect(u.hostname, u.port or '21') - username = u.username or self.FTP_USERNAME - password = u.password or self.FTP_PASSWORD - self.ftp.login(username, password) + u = urlparse(uri) + self.port = u.port + self.host = u.hostname + self.port = int(u.port or '21') + self.username = u.username or self.FTP_USERNAME + self.password = u.password or self.FTP_PASSWORD self.basedir = u.path.rstrip('/') def persist_file(self, path, buf, info, meta=None, headers=None): - buf.seek(0) - # If the path is like 'x/y/z.ext' the 'x/y' is rel_path and - # 'z.ext' is file name - # If path is only the file name 'z.ext', then rel_path is - # the empty string and filename is 'z.ext' - x = path.rsplit('/',1) - rel_path, filename = ('/' + x[0].lstrip('/'), x[1]) if len(x) > 1 else ('', x[0]) - abs_path = self.basedir + rel_path - ftp_makedirs_cwd(self.ftp, abs_path) - return threads.deferToThread( - self.ftp.storbinary, - 'STOR %s' % filename, - buf - ) + + def _persist_file(path, buf): + ftp = FTP() + ftp.connect(self.host, self.port) + ftp.login(self.username, self.password) + buf.seek(0) + # If the path is like 'x/y/z.ext' the 'x/y' is rel_path and + # 'z.ext' is file name + # If path is only the file name 'z.ext', then rel_path is + # the empty string and filename is 'z.ext' + x = path.rsplit('/',1) + rel_path, filename = ('/' + x[0].lstrip('/'), x[1]) if len(x) > 1 else ('', x[0]) + abs_path = self.basedir + rel_path + ftp_makedirs_cwd(ftp, abs_path) + ftp.storbinary('STOR %s' % filename, buf) + + return threads.deferToThread(_persist_file, path, buf) def stat_file(self, path, info): def _stat_file(path): From 8c970c636eb37deeb5caddbe5069bfcc0a79015e Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Wed, 21 Aug 2019 18:28:36 +0200 Subject: [PATCH 06/81] port from str to int MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- scrapy/pipelines/files.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 2959179b8..bbe4b9558 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -260,7 +260,7 @@ class FTPFilesStore(object): u = urlparse(uri) self.port = u.port self.host = u.hostname - self.port = int(u.port or '21') + self.port = int(u.port or 21) self.username = u.username or self.FTP_USERNAME self.password = u.password or self.FTP_PASSWORD self.basedir = u.path.rstrip('/') @@ -512,4 +512,4 @@ class FilesPipeline(MediaPipeline): def file_path(self, request, response=None, info=None): media_guid = hashlib.sha1(to_bytes(request.url)).hexdigest() media_ext = os.path.splitext(request.url)[1] - return 'full/%s%s' % (media_guid, media_ext) \ No newline at end of file + return 'full/%s%s' % (media_guid, media_ext) From bd22b25ef4e4223aafc6e326058c6d62b1fbf13c Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Thu, 22 Aug 2019 01:30:15 +0200 Subject: [PATCH 07/81] Make `stat_file` thread safe .. Refactor file storing.. Support act/psv --- scrapy/extensions/feedexport.py | 17 +++++--------- scrapy/pipelines/files.py | 39 +++++++++++++++------------------ scrapy/utils/ftp.py | 21 +++++++++++++++++- 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index d35551fdd..1ddc55f93 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -19,7 +19,7 @@ from twisted.internet import defer, threads from w3lib.url import file_uri_to_path from scrapy import signals -from scrapy.utils.ftp import ftp_makedirs_cwd +from scrapy.utils.ftp import ftp_makedirs_cwd, ftp_store_file from scrapy.exceptions import NotConfigured from scrapy.utils.misc import create_instance, load_object from scrapy.utils.log import failure_to_exc_info @@ -174,16 +174,11 @@ class FTPFeedStorage(BlockingFeedStorage): ) def _store_in_thread(self, file): - file.seek(0) - ftp = FTP() - ftp.connect(self.host, self.port) - ftp.login(self.username, self.password) - if self.use_active_mode: - ftp.set_pasv(False) - dirname, filename = posixpath.split(self.path) - ftp_makedirs_cwd(ftp, dirname) - ftp.storbinary('STOR %s' % filename, file) - ftp.quit() + ftp_store_file( + self.path, file, self.host, + self.port, self.username, + self.password, self.use_active_mode + ) class SpiderSlot(object): diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index bbe4b9558..04fbf3237 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -32,7 +32,7 @@ from scrapy.utils.python import to_bytes from scrapy.utils.request import referer_str from scrapy.utils.boto import is_botocore from scrapy.utils.datatypes import CaselessDict -from scrapy.utils.ftp import ftp_makedirs_cwd +from scrapy.utils.ftp import ftp_makedirs_cwd, ftp_store_file logger = logging.getLogger(__name__) @@ -254,6 +254,7 @@ class FTPFilesStore(object): FTP_USERNAME = None FTP_PASSWORD = None + USE_ACTIVE_MODE = None def __init__(self, uri): assert uri.startswith('ftp://') @@ -265,31 +266,26 @@ class FTPFilesStore(object): self.password = u.password or self.FTP_PASSWORD self.basedir = u.path.rstrip('/') - def persist_file(self, path, buf, info, meta=None, headers=None): - - def _persist_file(path, buf): - ftp = FTP() - ftp.connect(self.host, self.port) - ftp.login(self.username, self.password) - buf.seek(0) - # If the path is like 'x/y/z.ext' the 'x/y' is rel_path and - # 'z.ext' is file name - # If path is only the file name 'z.ext', then rel_path is - # the empty string and filename is 'z.ext' - x = path.rsplit('/',1) - rel_path, filename = ('/' + x[0].lstrip('/'), x[1]) if len(x) > 1 else ('', x[0]) - abs_path = self.basedir + rel_path - ftp_makedirs_cwd(ftp, abs_path) - ftp.storbinary('STOR %s' % filename, buf) - - return threads.deferToThread(_persist_file, path, buf) + def persist_file(self, path, buf, info, meta=None, headers=None): + path = '%s/%s' % (self.basedir, path) + return threads.deferToThread( + ftp_store_file, path,buf, + self.host, self.port,self.username, + self.password, self.USE_ACTIVE_MODE + ) def stat_file(self, path, info): def _stat_file(path): try: - last_modified = float(self.ftp.voidcmd("MDTM " + self.basedir + '/' + path)[4:].strip()) + ftp = FTP() + ftp.connect(self.host, self.port) + ftp.login(self.username, self.password) + if self.USE_ACTIVE_MODE: + ftp.set_pasv(False) + file_path = "%s/%s" % (self.basedir, path) + last_modified = float(ftp.voidcmd("MDTM %s" % file_path)[4:].strip()) m = hashlib.md5() - self.ftp.retrbinary('RETR %s' % self.basedir + path, m.update) + ftp.retrbinary('RETR %s' % file_path, m.update) return {'last_modified': last_modified, 'checksum': m.hexdigest()} # The file doesn't exist except Exception as e : @@ -372,6 +368,7 @@ class FilesPipeline(MediaPipeline): ftp_store = cls.STORE_SCHEMES['ftp'] ftp_store.FTP_USERNAME = settings['FTP_USER'] # Default is 'anonymous' ftp_store.FTP_PASSWORD = settings['FTP_PASSWORD'] # Default is `guest` + ftp_store.USE_ACTIVE_MODE = settings.getbool('FEED_STORAGE_FTP_ACTIVE') store_uri = settings['FILES_STORE'] return cls(store_uri, settings=settings) diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index 9eca6a4da..ba94ec142 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -1,4 +1,6 @@ -from ftplib import error_perm +import posixpath + +from ftplib import error_perm, FTP from posixpath import dirname def ftp_makedirs_cwd(ftp, path, first_call=True): @@ -13,3 +15,20 @@ def ftp_makedirs_cwd(ftp, path, first_call=True): ftp.mkd(path) if first_call: ftp.cwd(path) + +def ftp_store_file( + path, file, host ,port, + username, password, use_active_mode=False): + """Opens a FTP connection with passed credentials,sets current directory + to the directory extracted from given path, then uploads the file to server + """ + ftp = FTP() + ftp.connect(host, port) + ftp.login(username, password) + if use_active_mode: + ftp.set_pasv(False) + file.seek(0) + dirname, filename = posixpath.split(path) + ftp_makedirs_cwd(ftp, dirname) + ftp.storbinary('STOR %s' % filename, file) + ftp.quit() \ No newline at end of file From 2047124b3573d02ec60b13614f4e3dce85e71546 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Thu, 22 Aug 2019 16:18:14 +0200 Subject: [PATCH 08/81] Follow PEP8 .. Remove unnecessary comments --- scrapy/pipelines/files.py | 6 ++++-- scrapy/utils/ftp.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 04fbf3237..5780f63bd 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -330,6 +330,7 @@ class FilesPipeline(MediaPipeline): if isinstance(settings, dict) or settings is None: settings = Settings(settings) + cls_name = "FilesPipeline" self.store = self._get_store(store_uri) resolve = functools.partial(self._key_for_pipe, @@ -348,6 +349,7 @@ class FilesPipeline(MediaPipeline): self.files_result_field = settings.get( resolve('FILES_RESULT_FIELD'), self.FILES_RESULT_FIELD ) + super(FilesPipeline, self).__init__(download_func=download_func, settings=settings) @classmethod @@ -366,8 +368,8 @@ class FilesPipeline(MediaPipeline): gcs_store.POLICY = settings['FILES_STORE_GCS_ACL'] or None ftp_store = cls.STORE_SCHEMES['ftp'] - ftp_store.FTP_USERNAME = settings['FTP_USER'] # Default is 'anonymous' - ftp_store.FTP_PASSWORD = settings['FTP_PASSWORD'] # Default is `guest` + ftp_store.FTP_USERNAME = settings['FTP_USER'] + ftp_store.FTP_PASSWORD = settings['FTP_PASSWORD'] ftp_store.USE_ACTIVE_MODE = settings.getbool('FEED_STORAGE_FTP_ACTIVE') store_uri = settings['FILES_STORE'] diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index ba94ec142..bf67b9976 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -31,4 +31,4 @@ def ftp_store_file( dirname, filename = posixpath.split(path) ftp_makedirs_cwd(ftp, dirname) ftp.storbinary('STOR %s' % filename, file) - ftp.quit() \ No newline at end of file + ftp.quit() From 97d2f717ae30f53282a9cffacc40a879989f05af Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Thu, 22 Aug 2019 16:19:01 +0200 Subject: [PATCH 09/81] Support extracting ftp settings in `ImagesPipeline` --- scrapy/pipelines/images.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index fa4d12ad1..872342fc0 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -99,6 +99,11 @@ class ImagesPipeline(FilesPipeline): gcs_store.GCS_PROJECT_ID = settings['GCS_PROJECT_ID'] gcs_store.POLICY = settings['IMAGES_STORE_GCS_ACL'] or None + ftp_store = cls.STORE_SCHEMES['ftp'] + ftp_store.FTP_USERNAME = settings['FTP_USER'] + ftp_store.FTP_PASSWORD = settings['FTP_PASSWORD'] + ftp_store.USE_ACTIVE_MODE = settings.getbool('FEED_STORAGE_FTP_ACTIVE') + store_uri = settings['IMAGES_STORE'] return cls(store_uri, settings=settings) From 0e8770a2f4e96ee18e2fd0fc7bfb5f9bcd2f623d Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Fri, 6 Sep 2019 15:47:57 +0200 Subject: [PATCH 10/81] test for files pipeline ftp store --- scrapy/utils/test.py | 18 ++++++++++++++++++ tests/test_pipeline_files.py | 25 ++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 4b935c51b..59467f105 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -3,6 +3,7 @@ This module contains some assorted functions used in tests """ from __future__ import absolute_import +from posixpath import split import os from importlib import import_module @@ -61,6 +62,23 @@ def get_gcs_content_and_delete(bucket, path): bucket.delete_blob(path) return content, acl, blob +def get_ftp_content_and_delete(path, host ,port, + username, password, use_active_mode=False): + from ftplib import FTP + ftp = FTP() + ftp.connect(host, port) + ftp.login(username, password) + if use_active_mode: + ftp.set_pasv(False) + ftp_data = [] + def buffer_data(data): + ftp_data.append(data) + ftp.retrbinary('RETR %s' % path, buffer_data) + dirname, filename = split(path) + ftp.cwd(dirname) + ftp.delete(filename) + return "".join(ftp_data) + def get_crawler(spidercls=None, settings_dict=None): """Return an unconfigured Crawler object. If settings_dict is given, it will be used to populate the crawler settings with a project level diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 0c5aaaa44..000c1e2e2 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -11,13 +11,14 @@ from six import BytesIO from twisted.trial import unittest from twisted.internet import defer -from scrapy.pipelines.files import FilesPipeline, FSFilesStore, S3FilesStore, GCSFilesStore +from scrapy.pipelines.files import FilesPipeline, FSFilesStore, S3FilesStore, GCSFilesStore, FTPFilesStore from scrapy.item import Item, Field from scrapy.http import Request, Response from scrapy.settings import Settings from scrapy.utils.python import to_bytes from scrapy.utils.test import assert_aws_environ, get_s3_content_and_delete from scrapy.utils.test import assert_gcs_environ, get_gcs_content_and_delete +from scrapy.utils.test import get_ftp_content_and_delete from scrapy.utils.boto import is_botocore from tests import mock @@ -365,6 +366,28 @@ class TestGCSFilesStore(unittest.TestCase): self.assertEqual(blob.content_type, 'application/octet-stream') self.assertIn(expected_policy, acl) +class TestFTPFileStore(unittest.TestCase): + @defer.inlineCallbacks + def test_persist(self): + uri = os.environ.get('FTP_TEST_FILE_URI') + if not uri: + raise unittest.SkipTest("No FTP URI available for testing") + data = b"TestFTPFilesStore: \xe2\x98\x83" + buf = BytesIO(data) + meta = {'foo': 'bar'} + path = 'full/filename' + store = FTPFilesStore(uri) + empty_dict = yield store.stat_file(path, info=None) + self.assertEqual(empty_dict, {}) + yield store.persist_file(path, buf, info=None, meta=meta, headers=None) + stat = yield store.stat_file(path, info=None) + self.assertIn('last_modified', stat) + self.assertIn('checksum', stat) + self.assertEqual(stat['checksum'], 'd113d66b2ec7258724a268bd88eef6b6') + path = '%s/%s' % (store.basedir, path) + content = get_ftp_content_and_delete(path, store.host, store.port, + store.username, store.password, store.USE_ACTIVE_MODE) + self.assertEqual(data.decode(), content) class ItemWithFiles(Item): file_urls = Field() From b14c3cb612becc28499409202e904c062745d52c Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Thu, 19 Sep 2019 23:33:57 +0200 Subject: [PATCH 11/81] Add media pipelines FTP documentation --- docs/topics/media-pipeline.rst | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 0ce431ff5..d3fed928c 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -148,6 +148,27 @@ Where: * ``full`` is a sub-directory to separate full images from thumbnails (if used). For more info see :ref:`topics-images-thumbnails`. +FTP server storage +------------------ + +.. setting:: FTP_USER +.. setting:: FTP_PASSWORD + +:setting:`FILES_STORE` and :setting:`IMAGES_STORE` can represent a FTP server. +Scrapy will automatically upload the files to the server. + +:setting:`FILES_STORE` value: should be written in the form +`ftp://username:password@address:port/path` or `ftp://address:port/path`. In +the second case, the `username` and `password` are taken from `FTP_USER` and +`FTP_PASSWORD` settings respectively. + +.. note:: + The `path` can be left empty + +FTP supports two different connection modes: active or passive. Scrapy uses +the passive connection mode by default. To use the active connection mode instead, +set the `FEED_STORAGE_FTP_ACTIVE` setting to True. + Amazon S3 storage ----------------- From 28005b2872b897d84343d1e145fe50be880e91ff Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Sat, 28 Sep 2019 06:21:14 +0200 Subject: [PATCH 12/81] Update media-pipeline.rst --- docs/topics/media-pipeline.rst | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index d3fed928c..ceac317c0 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -151,23 +151,21 @@ Where: FTP server storage ------------------ -.. setting:: FTP_USER -.. setting:: FTP_PASSWORD - -:setting:`FILES_STORE` and :setting:`IMAGES_STORE` can represent a FTP server. +:setting:`FILES_STORE` and :setting:`IMAGES_STORE` can point to an FTP server. Scrapy will automatically upload the files to the server. -:setting:`FILES_STORE` value: should be written in the form -`ftp://username:password@address:port/path` or `ftp://address:port/path`. In -the second case, the `username` and `password` are taken from `FTP_USER` and -`FTP_PASSWORD` settings respectively. +:setting:`FILES_STORE` and :setting:`IMAGES_STORE` should be written in one of the +following forms:: -.. note:: - The `path` can be left empty + ftp://username:password@address:port/path + ftp://address:port/path + +If ``username`` and ``password`` are not provided, they are taken from :setting:`FTP_USER` and +:setting:`FTP_PASSWORD` settings respectively. FTP supports two different connection modes: active or passive. Scrapy uses the passive connection mode by default. To use the active connection mode instead, -set the `FEED_STORAGE_FTP_ACTIVE` setting to True. +set the :setting:`FEED_STORAGE_FTP_ACTIVE` setting to ``True``. Amazon S3 storage ----------------- From 175cd2ece5b9a8e2c735697e7a49d0baffc7cd52 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Tue, 1 Oct 2019 07:27:31 +0200 Subject: [PATCH 13/81] Update docs/topics/media-pipeline.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- docs/topics/media-pipeline.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index ceac317c0..327517996 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -160,7 +160,7 @@ following forms:: ftp://username:password@address:port/path ftp://address:port/path -If ``username`` and ``password`` are not provided, they are taken from :setting:`FTP_USER` and +If ``username`` and ``password`` are not provided, they are taken from the :setting:`FTP_USER` and :setting:`FTP_PASSWORD` settings respectively. FTP supports two different connection modes: active or passive. Scrapy uses From 5f168cd459cfc026de1e4d0b43c45ea740fe1dd5 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 2 Oct 2019 14:08:08 -0300 Subject: [PATCH 14/81] Response.follow_all --- docs/topics/request-response.rst | 4 ++ scrapy/http/response/__init__.py | 63 +++++++++++++++++----- scrapy/http/response/text.py | 61 ++++++++++++++++++--- tests/test_http_response.py | 91 ++++++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 20 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 727c67482..9fe3c7518 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -701,6 +701,8 @@ Response objects .. automethod:: Response.follow + .. automethod:: Response.follow_all + .. _urlparse.urljoin: https://docs.python.org/2/library/urlparse.html#urlparse.urljoin @@ -790,6 +792,8 @@ TextResponse objects .. automethod:: TextResponse.follow + .. automethod:: TextResponse.follow_all + .. method:: TextResponse.body_as_unicode() The same as :attr:`text`, but available as a method. This method is diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index b0a526b72..96359705e 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -113,8 +113,8 @@ class Response(object_ref): It accepts the same arguments as ``Request.__init__`` method, but ``url`` can be a relative URL or a ``scrapy.link.Link`` object, not only an absolute URL. - - :class:`~.TextResponse` provides a :meth:`~.TextResponse.follow` + + :class:`~.TextResponse` provides a :meth:`~.TextResponse.follow` method which supports selectors in addition to absolute/relative URLs and Link objects. """ @@ -123,14 +123,51 @@ class Response(object_ref): elif url is None: raise ValueError("url can't be None") url = self.urljoin(url) - return Request(url, callback, - method=method, - headers=headers, - body=body, - cookies=cookies, - meta=meta, - encoding=encoding, - priority=priority, - dont_filter=dont_filter, - errback=errback, - cb_kwargs=cb_kwargs) + return Request( + url=url, + callback=callback, + method=method, + headers=headers, + body=body, + cookies=cookies, + meta=meta, + encoding=encoding, + priority=priority, + dont_filter=dont_filter, + errback=errback, + cb_kwargs=cb_kwargs, + ) + + def follow_all(self, urls, callback=None, method='GET', headers=None, body=None, + cookies=None, meta=None, encoding='utf-8', priority=0, + dont_filter=False, errback=None, cb_kwargs=None): + # type: (...) -> Generator[Request, None, None] + """ + Return an iterable of :class:`~.Request` instance to follow all links + in ``urls``. It accepts the same arguments as ``Request.__init__`` method, + but elements of ``urls`` can be relative URLs or ``scrapy.link.Link`` objects + not only absolute URLs. + + :class:`~.TextResponse` provides a :meth:`~.TextResponse.follow_all` + method which supports selectors in addition to absolute/relative URLs + and Link objects. + """ + if not hasattr(urls, '__iter__'): + raise TypeError("'urls' argument must be an iterable") + return ( + self.follow( + url=url, + callback=callback, + method=method, + headers=headers, + body=body, + cookies=cookies, + meta=meta, + encoding=encoding, + priority=priority, + dont_filter=dont_filter, + errback=errback, + cb_kwargs=cb_kwargs, + ) + for url in urls + ) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 339913d4e..81e3f9b28 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -13,7 +13,6 @@ from w3lib.encoding import html_to_unicode, resolve_encoding, \ html_body_declared_encoding, http_content_type_encoding from w3lib.html import strip_html5_whitespace -from scrapy.http.request import Request from scrapy.http.response import Response from scrapy.utils.response import get_base_url from scrapy.utils.python import memoizemethod_noargs, to_native_str @@ -44,7 +43,7 @@ class TextResponse(Response): if isinstance(body, six.text_type): if self._encoding is None: raise TypeError('Cannot convert unicode body - %s has no encoding' % - type(self).__name__) + type(self).__name__) self._body = body.encode(self._encoding) else: super(TextResponse, self)._set_body(body) @@ -90,8 +89,8 @@ class TextResponse(Response): if self._cached_benc is None: content_type = to_native_str(self.headers.get(b'Content-Type', b'')) benc, ubody = html_to_unicode(content_type, self.body, - auto_detect_fun=self._auto_detect_fun, - default_encoding=self._DEFAULT_ENCODING) + auto_detect_fun=self._auto_detect_fun, + default_encoding=self._DEFAULT_ENCODING) self._cached_benc = benc self._cached_ubody = ubody return self._cached_benc @@ -129,7 +128,7 @@ class TextResponse(Response): Return a :class:`~.Request` instance to follow a link ``url``. It accepts the same arguments as ``Request.__init__`` method, but ``url`` can be not only an absolute URL, but also - + * a relative URL; * a scrapy.link.Link object (e.g. a link extractor result); * an attribute Selector (not SelectorList) - e.g. @@ -137,7 +136,7 @@ class TextResponse(Response): ``response.xpath('//img/@src')[0]``. * a Selector for ```` or ```` element, e.g. ``response.css('a.my_link')[0]``. - + See :ref:`response-follow-example` for usage examples. """ if isinstance(url, parsel.Selector): @@ -145,7 +144,9 @@ class TextResponse(Response): elif isinstance(url, parsel.SelectorList): raise ValueError("SelectorList is not supported") encoding = self.encoding if encoding is None else encoding - return super(TextResponse, self).follow(url, callback, + return super(TextResponse, self).follow( + url=url, + callback=callback, method=method, headers=headers, body=body, @@ -158,6 +159,52 @@ class TextResponse(Response): cb_kwargs=cb_kwargs, ) + def follow_all(self, urls=None, callback=None, method='GET', headers=None, body=None, + cookies=None, meta=None, encoding=None, priority=0, + dont_filter=False, errback=None, cb_kwargs=None, + css=None, xpath=None): + # type: (...) -> Generator[Request, None, None] + """ + A generator that produces :class:`~.Request` instances to follow all + links in ``urls``. It accepts the same arguments as the :class:`~.Request` + initializer, except that each ``urls`` element does not need to be an absolute + URL, it can be any of the following: + + * a relative URL; + * a scrapy.link.Link object (e.g. a link extractor result); + * an attribute Selector (not SelectorList) - e.g. + ``response.css('a::attr(href)')[0]`` or + ``response.xpath('//img/@src')[0]``. + * a Selector for ```` or ```` element, e.g. + ``response.css('a.my_link')[0]``. + + In addition, ``css`` and ``xpath`` arguments are accepted to perform the link extraction + within the ``follow_all`` method (only one of ``urls``, ``css`` and ``xpath`` are accepted). + """ + if len(list(filter(None, (urls, css, xpath)))) > 1: + raise ValueError('Please supply only one of the following arguments: {urls, css, xpath}') + if css: + urls = self.css(css) + elif xpath: + urls = self.xpath(xpath) + return ( + self.follow( + url=url, + callback=callback, + method=method, + headers=headers, + body=body, + cookies=cookies, + meta=meta, + encoding=encoding, + priority=priority, + dont_filter=dont_filter, + errback=errback, + cb_kwargs=cb_kwargs, + ) + for url in urls + ) + def _url_from_selector(sel): # type: (parsel.Selector) -> str diff --git a/tests/test_http_response.py b/tests/test_http_response.py index cd5c3486e..134856c78 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -143,6 +143,8 @@ class BaseResponseTest(unittest.TestCase): r.css('body') r.xpath('//body') + # Response.follow + def test_follow_url_absolute(self): self._assert_followed_url('http://foo.example.com', 'http://foo.example.com') @@ -166,6 +168,64 @@ class BaseResponseTest(unittest.TestCase): def test_follow_whitespace_link(self): self._assert_followed_url(Link('http://example.com/foo '), 'http://example.com/foo%20') + + # Response.follow_all + + def test_follow_all_absolute(self): + url_list = ['http://example.org', 'http://www.example.org', + 'http://example.com', 'http://www.example.com'] + self._assert_followed_all_urls(url_list, url_list) + + def test_follow_all_relative(self): + relative = ['foo', 'bar', 'foo/bar', 'bar/foo'] + absolute = [ + 'http://example.com/foo', + 'http://example.com/bar', + 'http://example.com/foo/bar', + 'http://example.com/bar/foo', + ] + self._assert_followed_all_urls(relative, absolute) + + def test_follow_all_links(self): + absolute = [ + 'http://example.com/foo', + 'http://example.com/bar', + 'http://example.com/foo/bar', + 'http://example.com/bar/foo', + ] + links = map(Link, absolute) + self._assert_followed_all_urls(links, absolute) + + def test_follow_all_invalid(self): + r = self.response_class("http://example.com") + with self.assertRaises(TypeError): + list(r.follow_all(None)) + with self.assertRaises(TypeError): + list(r.follow_all(12345)) + with self.assertRaises(ValueError): + list(r.follow_all([None])) + + def test_follow_all_whitespace(self): + relative = ['foo ', 'bar ', 'foo/bar ', 'bar/foo '] + absolute = [ + 'http://example.com/foo%20', + 'http://example.com/bar%20', + 'http://example.com/foo/bar%20', + 'http://example.com/bar/foo%20', + ] + self._assert_followed_all_urls(relative, absolute) + + def test_follow_all_whitespace_links(self): + absolute = [ + 'http://example.com/foo ', + 'http://example.com/bar ', + 'http://example.com/foo/bar ', + 'http://example.com/bar/foo ', + ] + links = map(Link, absolute) + expected = [u.replace(' ', '%20') for u in absolute] + self._assert_followed_all_urls(links, expected) + def _assert_followed_url(self, follow_obj, target_url, response=None): if response is None: response = self._links_response() @@ -173,6 +233,14 @@ class BaseResponseTest(unittest.TestCase): self.assertEqual(req.url, target_url) return req + def _assert_followed_all_urls(self, follow_obj, target_urls, response=None): + if response is None: + response = self._links_response() + followed = response.follow_all(follow_obj) + for req, target in zip(followed, target_urls): + self.assertEqual(req.url, target) + yield req + def _links_response(self): body = get_testdata('link_extractor', 'sgml_linkextractor.html') resp = self.response_class('http://example.com/index', body=body) @@ -483,6 +551,29 @@ class TextResponseTest(BaseResponseTest): ) self.assertEqual(req.encoding, 'cp1251') + def test_follow_all_css(self): + expected = [ + 'http://example.com/sample3.html', + 'http://example.com/innertag.html', + ] + response = self._links_response() + extracted = [r.url for r in response.follow_all(css='a[href*="example.com"]')] + self.assertEqual(expected, extracted) + + def test_follow_all_xpath(self): + expected = [ + 'http://example.com/sample3.html', + 'http://example.com/innertag.html', + ] + response = self._links_response() + extracted = response.follow_all(xpath='//a[contains(@href, "example.com")]') + self.assertEqual(expected, [r.url for r in extracted]) + + def test_follow_all_exception(self): + response = self._links_response() + with self.assertRaises(ValueError): + response.follow_all(css='a[href*="example.com"]', xpath='//a[contains(@href, "example.com")]') + class HtmlResponseTest(TextResponseTest): From e1fa1fd8ad62cb8958d21d3a33648ed4de6af757 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 10 Oct 2019 00:36:38 -0300 Subject: [PATCH 15/81] TextResponse.follow_all: skip invalid links --- scrapy/http/response/text.py | 26 +++++++--- .../sgml_linkextractor_no_href.html | 25 ++++++++++ tests/test_http_response.py | 47 ++++++++++++++++--- 3 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 tests/sample_data/link_extractor/sgml_linkextractor_no_href.html diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 81e3f9b28..ccacce550 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -180,13 +180,27 @@ class TextResponse(Response): In addition, ``css`` and ``xpath`` arguments are accepted to perform the link extraction within the ``follow_all`` method (only one of ``urls``, ``css`` and ``xpath`` are accepted). + + Note that when using the ``css`` or ``xpath`` parameters, this method will not produce + requests for selectors from which links cannot be obtained (for instance, anchor tags + without ``href`` attribute) """ - if len(list(filter(None, (urls, css, xpath)))) > 1: - raise ValueError('Please supply only one of the following arguments: {urls, css, xpath}') - if css: - urls = self.css(css) - elif xpath: - urls = self.xpath(xpath) + arg_count = len(list(filter(None, (urls, css, xpath)))) + if arg_count != 1: + raise ValueError('Please supply exactly one of the following arguments: {urls, css, xpath}') + if not urls: + urls = [] + if css: + selector_method = getattr(self, 'css') + expression = css + elif xpath: + selector_method = getattr(self, 'xpath') + expression = xpath + for selector in selector_method(expression): + try: + urls.append(_url_from_selector(selector)) + except ValueError: + pass return ( self.follow( url=url, diff --git a/tests/sample_data/link_extractor/sgml_linkextractor_no_href.html b/tests/sample_data/link_extractor/sgml_linkextractor_no_href.html new file mode 100644 index 000000000..0b01cede8 --- /dev/null +++ b/tests/sample_data/link_extractor/sgml_linkextractor_no_href.html @@ -0,0 +1,25 @@ + + + + Sample page with anchor tags containing no href attribute, to test the TextResponse.follow_all method + + + +
+ “The world as we have created it is a process of our + thinking. It cannot be changed without changing our thinking.” + + by Albert Einstein + (about) + + +
+ + + \ No newline at end of file diff --git a/tests/test_http_response.py b/tests/test_http_response.py index 134856c78..6d3c5cb9d 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -198,12 +198,20 @@ class BaseResponseTest(unittest.TestCase): def test_follow_all_invalid(self): r = self.response_class("http://example.com") - with self.assertRaises(TypeError): - list(r.follow_all(None)) - with self.assertRaises(TypeError): - list(r.follow_all(12345)) - with self.assertRaises(ValueError): - list(r.follow_all([None])) + if self.response_class == Response: + with self.assertRaises(TypeError): + list(r.follow_all(urls=None)) + with self.assertRaises(TypeError): + list(r.follow_all(urls=12345)) + with self.assertRaises(ValueError): + list(r.follow_all(urls=[None])) + else: + with self.assertRaises(ValueError): + list(r.follow_all(urls=None)) + with self.assertRaises(TypeError): + list(r.follow_all(urls=12345)) + with self.assertRaises(ValueError): + list(r.follow_all(urls=[None])) def test_follow_all_whitespace(self): relative = ['foo ', 'bar ', 'foo/bar ', 'bar/foo '] @@ -246,6 +254,11 @@ class BaseResponseTest(unittest.TestCase): resp = self.response_class('http://example.com/index', body=body) return resp + def _links_response_no_href(self): + body = get_testdata('link_extractor', 'sgml_linkextractor_no_href.html') + resp = self.response_class('http://example.com/index', body=body) + return resp + class TextResponseTest(BaseResponseTest): @@ -560,6 +573,16 @@ class TextResponseTest(BaseResponseTest): extracted = [r.url for r in response.follow_all(css='a[href*="example.com"]')] self.assertEqual(expected, extracted) + def test_follow_all_css_skip_invalid(self): + expected = [ + 'http://example.com/page/1/', + 'http://example.com/page/3/', + 'http://example.com/page/4/', + ] + response = self._links_response_no_href() + extracted = [r.url for r in response.follow_all(css='.pagination a')] + self.assertEqual(expected, extracted) + def test_follow_all_xpath(self): expected = [ 'http://example.com/sample3.html', @@ -569,7 +592,17 @@ class TextResponseTest(BaseResponseTest): extracted = response.follow_all(xpath='//a[contains(@href, "example.com")]') self.assertEqual(expected, [r.url for r in extracted]) - def test_follow_all_exception(self): + def test_follow_all_xpath_skip_invalid(self): + expected = [ + 'http://example.com/page/1/', + 'http://example.com/page/3/', + 'http://example.com/page/4/', + ] + response = self._links_response_no_href() + extracted = [r.url for r in response.follow_all(xpath='//div[@id="pagination"]/a')] + self.assertEqual(expected, extracted) + + def test_follow_all_too_many_arguments(self): response = self._links_response() with self.assertRaises(ValueError): response.follow_all(css='a[href*="example.com"]', xpath='//a[contains(@href, "example.com")]') From b970851299bf561af7ff0bb21caca9b6c3f296af Mon Sep 17 00:00:00 2001 From: elacuesta Date: Mon, 14 Oct 2019 13:35:06 -0300 Subject: [PATCH 16/81] Update scrapy/http/response/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- scrapy/http/response/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index 96359705e..cdaababac 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -143,7 +143,7 @@ class Response(object_ref): dont_filter=False, errback=None, cb_kwargs=None): # type: (...) -> Generator[Request, None, None] """ - Return an iterable of :class:`~.Request` instance to follow all links + Return an iterable of :class:`~.Request` instances to follow all links in ``urls``. It accepts the same arguments as ``Request.__init__`` method, but elements of ``urls`` can be relative URLs or ``scrapy.link.Link`` objects not only absolute URLs. From ba840c5a6bea95333b3b78fb767ac6f092d02177 Mon Sep 17 00:00:00 2001 From: elacuesta Date: Mon, 14 Oct 2019 13:35:24 -0300 Subject: [PATCH 17/81] Update scrapy/http/response/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- scrapy/http/response/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index cdaababac..92fa01621 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -145,7 +145,7 @@ class Response(object_ref): """ Return an iterable of :class:`~.Request` instances to follow all links in ``urls``. It accepts the same arguments as ``Request.__init__`` method, - but elements of ``urls`` can be relative URLs or ``scrapy.link.Link`` objects + but elements of ``urls`` can be relative URLs or :class:`~scrapy.link.Link` objects, not only absolute URLs. :class:`~.TextResponse` provides a :meth:`~.TextResponse.follow_all` From 498d33aac37d5d7a0955e9b409fc3d5ff8627dc0 Mon Sep 17 00:00:00 2001 From: elacuesta Date: Mon, 14 Oct 2019 13:35:54 -0300 Subject: [PATCH 18/81] Update scrapy/http/response/text.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- scrapy/http/response/text.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index ccacce550..bb5a4eb9d 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -183,7 +183,7 @@ class TextResponse(Response): Note that when using the ``css`` or ``xpath`` parameters, this method will not produce requests for selectors from which links cannot be obtained (for instance, anchor tags - without ``href`` attribute) + without an ``href`` attribute) """ arg_count = len(list(filter(None, (urls, css, xpath)))) if arg_count != 1: From c7c54f5453eaa16f2b6d6441be68b0a272606ec9 Mon Sep 17 00:00:00 2001 From: elacuesta Date: Mon, 14 Oct 2019 13:47:44 -0300 Subject: [PATCH 19/81] Update scrapy/http/response/text.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- scrapy/http/response/text.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index bb5a4eb9d..ecb582b4a 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -179,7 +179,7 @@ class TextResponse(Response): ``response.css('a.my_link')[0]``. In addition, ``css`` and ``xpath`` arguments are accepted to perform the link extraction - within the ``follow_all`` method (only one of ``urls``, ``css`` and ``xpath`` are accepted). + within the ``follow_all`` method (only one of ``urls``, ``css`` and ``xpath`` is accepted). Note that when using the ``css`` or ``xpath`` parameters, this method will not produce requests for selectors from which links cannot be obtained (for instance, anchor tags From 9d5398e7f2834940cbf9c2efa312bf5d96ffba98 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 14 Oct 2019 13:57:46 -0300 Subject: [PATCH 20/81] TextResponse.follow_all: improve docs --- scrapy/http/response/text.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index ecb582b4a..b2907baa4 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -129,13 +129,14 @@ class TextResponse(Response): It accepts the same arguments as ``Request.__init__`` method, but ``url`` can be not only an absolute URL, but also - * a relative URL; - * a scrapy.link.Link object (e.g. a link extractor result); - * an attribute Selector (not SelectorList) - e.g. + * a relative URL + * a :class:`~scrapy.link.Link` object, e.g. the result of + :ref:`topics-link-extractors` + * a :class:`~scrapy.selector.Selector` object for a ```` or ```` element, e.g. + ``response.css('a.my_link')[0]`` + * an attribute :class:`~scrapy.selector.Selector` (not SelectorList), e.g. ``response.css('a::attr(href)')[0]`` or - ``response.xpath('//img/@src')[0]``. - * a Selector for ```` or ```` element, e.g. - ``response.css('a.my_link')[0]``. + ``response.xpath('//img/@src')[0]`` See :ref:`response-follow-example` for usage examples. """ @@ -170,13 +171,14 @@ class TextResponse(Response): initializer, except that each ``urls`` element does not need to be an absolute URL, it can be any of the following: - * a relative URL; - * a scrapy.link.Link object (e.g. a link extractor result); - * an attribute Selector (not SelectorList) - e.g. + * a relative URL + * a :class:`~scrapy.link.Link` object, e.g. the result of + :ref:`topics-link-extractors` + * a :class:`~scrapy.selector.Selector` object for a ```` or ```` element, e.g. + ``response.css('a.my_link')[0]`` + * an attribute :class:`~scrapy.selector.Selector` (not SelectorList), e.g. ``response.css('a::attr(href)')[0]`` or - ``response.xpath('//img/@src')[0]``. - * a Selector for ```` or ```` element, e.g. - ``response.css('a.my_link')[0]``. + ``response.xpath('//img/@src')[0]`` In addition, ``css`` and ``xpath`` arguments are accepted to perform the link extraction within the ``follow_all`` method (only one of ``urls``, ``css`` and ``xpath`` is accepted). @@ -187,7 +189,7 @@ class TextResponse(Response): """ arg_count = len(list(filter(None, (urls, css, xpath)))) if arg_count != 1: - raise ValueError('Please supply exactly one of the following arguments: {urls, css, xpath}') + raise ValueError('Please supply exactly one of the following arguments: urls, css, xpath') if not urls: urls = [] if css: From 2a4d4a466aa6cede4829b62c7287332a627877ed Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Tue, 15 Oct 2019 11:52:12 -0300 Subject: [PATCH 21/81] TextResponse.follow_all: Simplify implementation --- scrapy/http/response/text.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index b2907baa4..25e115bf9 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -191,14 +191,12 @@ class TextResponse(Response): if arg_count != 1: raise ValueError('Please supply exactly one of the following arguments: urls, css, xpath') if not urls: - urls = [] if css: - selector_method = getattr(self, 'css') - expression = css - elif xpath: - selector_method = getattr(self, 'xpath') - expression = xpath - for selector in selector_method(expression): + selector_list = self.css(css) + if xpath: + selector_list = self.xpath(xpath) + urls = [] + for selector in selector_list: try: urls.append(_url_from_selector(selector)) except ValueError: From 2c6f7fee6456168f4870293c442258a2470b1b48 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Tue, 15 Oct 2019 13:48:14 -0300 Subject: [PATCH 22/81] TextResponse.follow_all: invoke Response.follow_all --- scrapy/http/response/text.py | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 25e115bf9..f782f6217 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -201,22 +201,19 @@ class TextResponse(Response): urls.append(_url_from_selector(selector)) except ValueError: pass - return ( - self.follow( - url=url, - callback=callback, - method=method, - headers=headers, - body=body, - cookies=cookies, - meta=meta, - encoding=encoding, - priority=priority, - dont_filter=dont_filter, - errback=errback, - cb_kwargs=cb_kwargs, - ) - for url in urls + return super(TextResponse, self).follow_all( + urls=urls, + callback=callback, + method=method, + headers=headers, + body=body, + cookies=cookies, + meta=meta, + encoding=encoding, + priority=priority, + dont_filter=dont_filter, + errback=errback, + cb_kwargs=cb_kwargs, ) From 6df6b6dd6a5afd9172c03f6b9e8ce8e180867339 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 21 Oct 2019 03:56:45 -0300 Subject: [PATCH 23/81] Initializer -> __init__ --- scrapy/http/response/text.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index f782f6217..74017b5aa 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -167,9 +167,9 @@ class TextResponse(Response): # type: (...) -> Generator[Request, None, None] """ A generator that produces :class:`~.Request` instances to follow all - links in ``urls``. It accepts the same arguments as the :class:`~.Request` - initializer, except that each ``urls`` element does not need to be an absolute - URL, it can be any of the following: + links in ``urls``. It accepts the same arguments as the :class:`~.Request`'s + ``__init__`` method, except that each ``urls`` element does not need to be + an absolute URL, it can be any of the following: * a relative URL * a :class:`~scrapy.link.Link` object, e.g. the result of From e6c5292a7c4391642897ee31ea2ded889b3b88dc Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 20 Nov 2019 09:29:55 -0300 Subject: [PATCH 24/81] Response.follow_all: Specific exception for invalid selectors --- scrapy/http/response/text.py | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 74017b5aa..5110b4bd4 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -5,17 +5,18 @@ discovering (through HTTP headers) to base Response class. See documentation in docs/topics/request-response.rst """ -import six -from six.moves.urllib.parse import urljoin +from contextlib import suppress import parsel -from w3lib.encoding import html_to_unicode, resolve_encoding, \ - html_body_declared_encoding, http_content_type_encoding +import six +from six.moves.urllib.parse import urljoin +from w3lib.encoding import (html_body_declared_encoding, html_to_unicode, + http_content_type_encoding, resolve_encoding) from w3lib.html import strip_html5_whitespace from scrapy.http.response import Response -from scrapy.utils.response import get_base_url from scrapy.utils.python import memoizemethod_noargs, to_native_str +from scrapy.utils.response import get_base_url class TextResponse(Response): @@ -197,10 +198,8 @@ class TextResponse(Response): selector_list = self.xpath(xpath) urls = [] for selector in selector_list: - try: + with suppress(_InvalidSelector): urls.append(_url_from_selector(selector)) - except ValueError: - pass return super(TextResponse, self).follow_all( urls=urls, callback=callback, @@ -217,18 +216,24 @@ class TextResponse(Response): ) +class _InvalidSelector(ValueError): + """ + Raised when a URL cannot be obtained from a Selector + """ + + def _url_from_selector(sel): # type: (parsel.Selector) -> str if isinstance(sel.root, six.string_types): # e.g. ::attr(href) result return strip_html5_whitespace(sel.root) if not hasattr(sel.root, 'tag'): - raise ValueError("Unsupported selector: %s" % sel) + raise _InvalidSelector("Unsupported selector: %s" % sel) if sel.root.tag not in ('a', 'link'): - raise ValueError("Only and elements are supported; got <%s>" % - sel.root.tag) + raise _InvalidSelector("Only and elements are supported; got <%s>" % + sel.root.tag) href = sel.root.get('href') if href is None: - raise ValueError("<%s> element has no href attribute: %s" % - (sel.root.tag, sel)) + raise _InvalidSelector("<%s> element has no href attribute: %s" % + (sel.root.tag, sel)) return strip_html5_whitespace(href) From b602c61e1cc00e04ce435078644d5ea0c0249903 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 20 Nov 2019 09:38:54 -0300 Subject: [PATCH 25/81] [Test] Rename outdated sample files --- .../{sgml_linkextractor.html => linkextractor.html} | 0 ..._linkextractor_no_href.html => linkextractor_no_href.html} | 0 tests/test_http_response.py | 4 ++-- 3 files changed, 2 insertions(+), 2 deletions(-) rename tests/sample_data/link_extractor/{sgml_linkextractor.html => linkextractor.html} (100%) rename tests/sample_data/link_extractor/{sgml_linkextractor_no_href.html => linkextractor_no_href.html} (100%) diff --git a/tests/sample_data/link_extractor/sgml_linkextractor.html b/tests/sample_data/link_extractor/linkextractor.html similarity index 100% rename from tests/sample_data/link_extractor/sgml_linkextractor.html rename to tests/sample_data/link_extractor/linkextractor.html diff --git a/tests/sample_data/link_extractor/sgml_linkextractor_no_href.html b/tests/sample_data/link_extractor/linkextractor_no_href.html similarity index 100% rename from tests/sample_data/link_extractor/sgml_linkextractor_no_href.html rename to tests/sample_data/link_extractor/linkextractor_no_href.html diff --git a/tests/test_http_response.py b/tests/test_http_response.py index 6d3c5cb9d..0ae1612b5 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -250,12 +250,12 @@ class BaseResponseTest(unittest.TestCase): yield req def _links_response(self): - body = get_testdata('link_extractor', 'sgml_linkextractor.html') + body = get_testdata('link_extractor', 'linkextractor.html') resp = self.response_class('http://example.com/index', body=body) return resp def _links_response_no_href(self): - body = get_testdata('link_extractor', 'sgml_linkextractor_no_href.html') + body = get_testdata('link_extractor', 'linkextractor_no_href.html') resp = self.response_class('http://example.com/index', body=body) return resp From 6f4e84ecf95feabe15fda0819bc428f8e0d4d340 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 20 Nov 2019 09:55:15 -0300 Subject: [PATCH 26/81] PEP8 adjustments for scrapy.http.response module --- scrapy/http/response/__init__.py | 6 ++++-- scrapy/http/response/html.py | 1 + scrapy/http/response/text.py | 2 ++ scrapy/http/response/xml.py | 1 + 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index 79a8d0ca0..b9e638551 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -4,6 +4,8 @@ responses in Scrapy. See documentation in docs/topics/request-response.rst """ +from typing import Generator + from six.moves.urllib.parse import urljoin from scrapy.http.request import Request @@ -41,8 +43,8 @@ class Response(object_ref): if isinstance(url, str): self._url = url else: - raise TypeError('%s url must be str, got %s:' % (type(self).__name__, - type(url).__name__)) + raise TypeError('%s url must be str, got %s:' % + (type(self).__name__, type(url).__name__)) url = property(_get_url, obsolete_setter(_set_url, 'url')) diff --git a/scrapy/http/response/html.py b/scrapy/http/response/html.py index bd3559fbb..7eed052c2 100644 --- a/scrapy/http/response/html.py +++ b/scrapy/http/response/html.py @@ -7,5 +7,6 @@ See documentation in docs/topics/request-response.rst from scrapy.http.response.text import TextResponse + class HtmlResponse(TextResponse): pass diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index b97420345..6acf1026f 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -6,6 +6,7 @@ See documentation in docs/topics/request-response.rst """ from contextlib import suppress +from typing import Generator import parsel import six @@ -14,6 +15,7 @@ from w3lib.encoding import (html_body_declared_encoding, html_to_unicode, http_content_type_encoding, resolve_encoding) from w3lib.html import strip_html5_whitespace +from scrapy.http import Request from scrapy.http.response import Response from scrapy.utils.python import memoizemethod_noargs, to_unicode from scrapy.utils.response import get_base_url diff --git a/scrapy/http/response/xml.py b/scrapy/http/response/xml.py index 1df33fee5..abf474a2f 100644 --- a/scrapy/http/response/xml.py +++ b/scrapy/http/response/xml.py @@ -7,5 +7,6 @@ See documentation in docs/topics/request-response.rst from scrapy.http.response.text import TextResponse + class XmlResponse(TextResponse): pass From 6781d2f5b261305a4f6aa41a2b485a2e5cc7bc76 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 20 Nov 2019 09:58:25 -0300 Subject: [PATCH 27/81] Update sample file references --- tests/test_linkextractors.py | 2 +- tests/test_linkextractors_deprecated.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 57ef1694a..0b94f937f 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -16,7 +16,7 @@ class Base: escapes_whitespace = False def setUp(self): - body = get_testdata('link_extractor', 'sgml_linkextractor.html') + body = get_testdata('link_extractor', 'linkextractor.html') self.response = HtmlResponse(url='http://example.com/index', body=body) def test_urls_type(self): diff --git a/tests/test_linkextractors_deprecated.py b/tests/test_linkextractors_deprecated.py index 1366971be..388ed6ad4 100644 --- a/tests/test_linkextractors_deprecated.py +++ b/tests/test_linkextractors_deprecated.py @@ -111,7 +111,7 @@ class BaseSgmlLinkExtractorTestCase(unittest.TestCase): class HtmlParserLinkExtractorTestCase(unittest.TestCase): def setUp(self): - body = get_testdata('link_extractor', 'sgml_linkextractor.html') + body = get_testdata('link_extractor', 'linkextractor.html') self.response = HtmlResponse(url='http://example.com/index', body=body) def test_extraction(self): @@ -183,7 +183,7 @@ class RegexLinkExtractorTestCase(unittest.TestCase): # than it should be. def setUp(self): - body = get_testdata('link_extractor', 'sgml_linkextractor.html') + body = get_testdata('link_extractor', 'linkextractor.html') self.response = HtmlResponse(url='http://example.com/index', body=body) def test_extraction(self): From dd12f5fdcd5ce2c63e9a5d3626031f5d93c1628e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 25 Nov 2019 15:59:59 +0100 Subject: [PATCH 28/81] Use Response.follow_all in the documentation where appropiate --- docs/intro/tutorial.rst | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index 6b15a5fbd..a2775e0bb 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -625,12 +625,12 @@ attribute automatically. So the code can be shortened further:: for a in response.css('li.next a'): yield response.follow(a, callback=self.parse) -.. note:: +To create multiple requests from an iterable, you can use +:meth:`response.follow_all ` instead:: + + links = response.css('li.next a') + yield from response.follow_all(links, callback=self.parse) - ``response.follow(response.css('li.next a'))`` is not valid because - ``response.css`` returns a list-like object with selectors for all results, - not a single selector. A ``for`` loop like in the example above, or - ``response.follow(response.css('li.next a')[0])`` is fine. More examples and patterns -------------------------- @@ -647,13 +647,11 @@ this time for scraping author information:: start_urls = ['http://quotes.toscrape.com/'] def parse(self, response): - # follow links to author pages - for href in response.css('.author + a::attr(href)'): - yield response.follow(href, self.parse_author) + author_page_links = response.css('.author + a') + yield from response.follow_all(author_links, self.parse_author) - # follow pagination links - for href in response.css('li.next a::attr(href)'): - yield response.follow(href, self.parse) + pagination_links = response.css('li.next a') + yield from response.follow_all(pagination_links, self.parse) def parse_author(self, response): def extract_with_css(query): @@ -669,8 +667,10 @@ This spider will start from the main page, it will follow all the links to the authors pages calling the ``parse_author`` callback for each of them, and also the pagination links with the ``parse`` callback as we saw before. -Here we're passing callbacks to ``response.follow`` as positional arguments -to make the code shorter; it also works for ``scrapy.Request``. +Here we're passing callbacks to +:meth:`response.follow_all ` as positional +arguments to make the code shorter; it also works for +:class:`~scrapy.http.Request`. The ``parse_author`` callback defines a helper function to extract and cleanup the data from a CSS query and yields the Python dict with the author data. From 5980b0f2840f51f8f9c7c3a266be93527999dd11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 2 Dec 2019 16:47:44 +0100 Subject: [PATCH 29/81] =?UTF-8?q?Don=E2=80=99t=20use=20follow=5Fall=20wher?= =?UTF-8?q?e=20a=20single=20item=20is=20expected=20(#4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/intro/tutorial.rst | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index a2775e0bb..2f97017fc 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -616,20 +616,19 @@ instance; you still have to yield this Request. You can also pass a selector to ``response.follow`` instead of a string; this selector should extract necessary attributes:: - for href in response.css('li.next a::attr(href)'): - yield response.follow(href, callback=self.parse) + href = response.css('li.next a::attr(href)')[0] + yield response.follow(href, callback=self.parse) For ```` elements there is a shortcut: ``response.follow`` uses their href attribute automatically. So the code can be shortened further:: - for a in response.css('li.next a'): - yield response.follow(a, callback=self.parse) + a = response.css('li.next a')[0] + yield response.follow(a, callback=self.parse) To create multiple requests from an iterable, you can use :meth:`response.follow_all ` instead:: - links = response.css('li.next a') - yield from response.follow_all(links, callback=self.parse) + yield from response.follow_all(response.css('a'), callback=self.parse) More examples and patterns From 3d77f74e4089c1a9700fb9e2bc62fc196176250c Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Tue, 5 Nov 2019 00:54:46 -0300 Subject: [PATCH 30/81] 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 31/81] 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 2a9f5a0aefae83fc0b1dc161d117a265148ad1ef Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Tue, 3 Dec 2019 15:56:50 -0300 Subject: [PATCH 32/81] Skip invalid links when passing SelectorLists to Response.follow_all --- scrapy/http/response/text.py | 19 +++++++++++-------- tests/test_http_response.py | 12 ++++++++---- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 6acf1026f..e3646b2d5 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -7,10 +7,10 @@ See documentation in docs/topics/request-response.rst from contextlib import suppress from typing import Generator +from urllib.parse import urljoin import parsel import six -from six.moves.urllib.parse import urljoin from w3lib.encoding import (html_body_declared_encoding, html_to_unicode, http_content_type_encoding, resolve_encoding) from w3lib.html import strip_html5_whitespace @@ -183,22 +183,25 @@ class TextResponse(Response): In addition, ``css`` and ``xpath`` arguments are accepted to perform the link extraction within the ``follow_all`` method (only one of ``urls``, ``css`` and ``xpath`` is accepted). - Note that when using the ``css`` or ``xpath`` parameters, this method will not produce - requests for selectors from which links cannot be obtained (for instance, anchor tags - without an ``href`` attribute) + Note that when passing a ``SelectorList`` as argument for the ``urls`` parameter or + using the ``css`` or ``xpath`` parameters, this method will not produce requests for + selectors from which links cannot be obtained (for instance, anchor tags without an + ``href`` attribute) """ arg_count = len(list(filter(None, (urls, css, xpath)))) if arg_count != 1: raise ValueError('Please supply exactly one of the following arguments: urls, css, xpath') if not urls: if css: - selector_list = self.css(css) + urls = self.css(css) if xpath: - selector_list = self.xpath(xpath) + urls = self.xpath(xpath) + if isinstance(urls, parsel.SelectorList): + selectors = urls urls = [] - for selector in selector_list: + for sel in selectors: with suppress(_InvalidSelector): - urls.append(_url_from_selector(selector)) + urls.append(_url_from_selector(sel)) return super(TextResponse, self).follow_all( urls=urls, callback=callback, diff --git a/tests/test_http_response.py b/tests/test_http_response.py index 36ccdfa1f..ce13650ce 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -579,8 +579,10 @@ class TextResponseTest(BaseResponseTest): 'http://example.com/page/4/', ] response = self._links_response_no_href() - extracted = [r.url for r in response.follow_all(css='.pagination a')] - self.assertEqual(expected, extracted) + extracted1 = [r.url for r in response.follow_all(css='.pagination a')] + self.assertEqual(expected, extracted1) + extracted2 = [r.url for r in response.follow_all(response.css('.pagination a'))] + self.assertEqual(expected, extracted2) def test_follow_all_xpath(self): expected = [ @@ -598,8 +600,10 @@ class TextResponseTest(BaseResponseTest): 'http://example.com/page/4/', ] response = self._links_response_no_href() - extracted = [r.url for r in response.follow_all(xpath='//div[@id="pagination"]/a')] - self.assertEqual(expected, extracted) + extracted1 = [r.url for r in response.follow_all(xpath='//div[@id="pagination"]/a')] + self.assertEqual(expected, extracted1) + extracted2 = [r.url for r in response.follow_all(response.xpath('//div[@id="pagination"]/a'))] + self.assertEqual(expected, extracted2) def test_follow_all_too_many_arguments(self): response = self._links_response() From 62778cf23f6e0cad5839f31f9da8b3c5778dcbdb Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 11 Sep 2019 15:40:44 -0300 Subject: [PATCH 33/81] Request: remove restriction about errback without callback --- scrapy/http/request/__init__.py | 1 - tests/test_http_request.py | 46 ++++++++++++++++++++++----------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 76a428199..61c0a4c9e 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -32,7 +32,6 @@ class Request(object_ref): raise TypeError('callback must be a callable, got %s' % type(callback).__name__) if errback is not None and not callable(errback): raise TypeError('errback must be a callable, got %s' % type(errback).__name__) - assert callback or not errback, "Cannot use errback without a callback" self.callback = callback self.errback = errback diff --git a/tests/test_http_request.py b/tests/test_http_request.py index 9df6ff67b..3449c7a40 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -246,25 +246,41 @@ class RequestTest(unittest.TestCase): self.assertRaises(AttributeError, setattr, r, 'url', 'http://example2.com') self.assertRaises(AttributeError, setattr, r, 'body', 'xxx') - def test_callback_is_callable(self): + def test_callback_and_errback(self): def a_function(): pass - r = self.request_class('http://example.com') - self.assertIsNone(r.callback) - r = self.request_class('http://example.com', a_function) - self.assertIs(r.callback, a_function) - with self.assertRaises(TypeError): - self.request_class('http://example.com', 'a_function') - def test_errback_is_callable(self): - def a_function(): - pass - r = self.request_class('http://example.com') - self.assertIsNone(r.errback) - r = self.request_class('http://example.com', a_function, errback=a_function) - self.assertIs(r.errback, a_function) + r1 = self.request_class('http://example.com') + self.assertIsNone(r1.callback) + self.assertIsNone(r1.errback) + + r2 = self.request_class('http://example.com', callback=a_function) + self.assertIs(r2.callback, a_function) + self.assertIsNone(r2.errback) + + r3 = self.request_class('http://example.com', errback=a_function) + self.assertIsNone(r3.callback) + self.assertIs(r3.errback, a_function) + + r4 = self.request_class( + url='http://example.com', + callback=a_function, + errback=a_function, + ) + self.assertIs(r4.callback, a_function) + self.assertIs(r4.errback, a_function) + + def test_callback_and_errback_type(self): with self.assertRaises(TypeError): - self.request_class('http://example.com', a_function, errback='a_function') + self.request_class('http://example.com', callback='a_function') + with self.assertRaises(TypeError): + self.request_class('http://example.com', errback='a_function') + with self.assertRaises(TypeError): + self.request_class( + url='http://example.com', + callback='a_function', + errback='a_function', + ) def test_from_curl(self): # Note: more curated tests regarding curl conversion are in From 931b7e68d33e06b624b49a7abeababb4e3540474 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 09:50:28 -0300 Subject: [PATCH 34/81] 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 35/81] 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 36/81] 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 37/81] 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 38/81] 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 39/81] 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 40/81] 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 41/81] 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 42/81] 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 43/81] 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 44/81] [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 45/81] 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) From 81175669746737f1ab0bd0236a70f787ed1855a4 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 16 Dec 2019 22:12:27 +0500 Subject: [PATCH 46/81] Add utils.defer.deferred_f_from_coro_f. --- scrapy/utils/defer.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index bbd5ebe52..62b43a96c 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -2,6 +2,7 @@ Helper functions for dealing with Twisted deferreds """ import asyncio +from functools import wraps import inspect from twisted.internet import defer, task @@ -140,3 +141,15 @@ def deferred_from_coro(o): # wrapping the coroutine into a Future and then into a Deferred, this requires AsyncioSelectorReactor return defer.Deferred.fromFuture(asyncio.ensure_future(o)) return o + + +def deferred_f_from_coro_f(coro_f): + """ Converts a coroutine function into a function that returns a Deferred. + + The coroutine function will be called at the time when the wrapper is called. Wrapper args will be passed to it. + This is useful for callback chains, as callback functions are called with the previous callback result. + """ + @wraps(coro_f) + def f(*coro_args, **coro_kwargs): + return deferred_from_coro(coro_f(*coro_args, **coro_kwargs)) + return f From 1f9cef787d3ca0c12099f1b1b4c52efc510e381d Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 10 Sep 2019 14:26:21 +0500 Subject: [PATCH 47/81] Add async def support to pipelines. --- scrapy/pipelines/__init__.py | 4 +++- tests/test_pipelines.py | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/scrapy/pipelines/__init__.py b/scrapy/pipelines/__init__.py index aa1bfb77f..1a45e00a2 100644 --- a/scrapy/pipelines/__init__.py +++ b/scrapy/pipelines/__init__.py @@ -6,6 +6,8 @@ See documentation in docs/item-pipeline.rst from scrapy.middleware import MiddlewareManager from scrapy.utils.conf import build_component_list +from scrapy.utils.defer import deferred_f_from_coro_f + class ItemPipelineManager(MiddlewareManager): @@ -19,7 +21,7 @@ class ItemPipelineManager(MiddlewareManager): def _add_middleware(self, pipe): super(ItemPipelineManager, self)._add_middleware(pipe) if hasattr(pipe, 'process_item'): - self.methods['process_item'].append(pipe.process_item) + self.methods['process_item'].append(deferred_f_from_coro_f(pipe.process_item)) def process_item(self, item, spider): return self._process_chain('process_item', item, spider) diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index bc53f5427..cfe4471d7 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -26,6 +26,13 @@ class DeferredPipeline: return d +class AsyncDefPipeline: + async def process_item(self, item, spider): + await defer.succeed(42) + item['pipeline_passed'] = True + return item + + class ItemSpider(Spider): name = 'itemspider' @@ -69,3 +76,9 @@ class PipelineTestCase(unittest.TestCase): crawler = self._create_crawler(DeferredPipeline) yield crawler.crawl(mockserver=self.mockserver) self.assertEqual(len(self.items), 1) + + @defer.inlineCallbacks + def test_asyncdef_pipeline(self): + crawler = self._create_crawler(AsyncDefPipeline) + yield crawler.crawl(mockserver=self.mockserver) + self.assertEqual(len(self.items), 1) From bfdd552a32b3a67dfa2895b483c187d87b63b50e Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 10 Sep 2019 14:57:07 +0500 Subject: [PATCH 48/81] Add a test for pipelines using asyncio. --- tests/test_pipelines.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index cfe4471d7..aba6d85b7 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -1,3 +1,5 @@ +import asyncio + from twisted.internet import defer from twisted.internet.defer import Deferred from twisted.trial import unittest @@ -33,6 +35,13 @@ class AsyncDefPipeline: return item +class AsyncDefAsyncioPipeline: + async def process_item(self, item, spider): + await asyncio.sleep(0.2) + item['pipeline_passed'] = True + return item + + class ItemSpider(Spider): name = 'itemspider' @@ -82,3 +91,9 @@ class PipelineTestCase(unittest.TestCase): crawler = self._create_crawler(AsyncDefPipeline) yield crawler.crawl(mockserver=self.mockserver) self.assertEqual(len(self.items), 1) + + @defer.inlineCallbacks + def test_asyncdef_asyncio_pipeline(self): + crawler = self._create_crawler(AsyncDefAsyncioPipeline) + yield crawler.crawl(mockserver=self.mockserver) + self.assertEqual(len(self.items), 1) From bdef948aaebced3d28ea7b81525ea9edf7cc650c Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 16 Dec 2019 23:15:43 +0500 Subject: [PATCH 49/81] Mark the asyncio pipelines test as only_asyncio. --- tests/test_pipelines.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index aba6d85b7..6f33282f2 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -1,5 +1,6 @@ import asyncio +from pytest import mark from twisted.internet import defer from twisted.internet.defer import Deferred from twisted.trial import unittest @@ -92,6 +93,7 @@ class PipelineTestCase(unittest.TestCase): yield crawler.crawl(mockserver=self.mockserver) self.assertEqual(len(self.items), 1) + @mark.only_asyncio() @defer.inlineCallbacks def test_asyncdef_asyncio_pipeline(self): crawler = self._create_crawler(AsyncDefAsyncioPipeline) From 9d8c54c0f2a98dc627efe8a2b58cfd311f2105cb Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 16 Dec 2019 22:43:55 +0500 Subject: [PATCH 50/81] Fix/ignore flake8 problems. --- pytest.ini | 1 + scrapy/pipelines/__init__.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index c3f3292bb..1030d5530 100644 --- a/pytest.ini +++ b/pytest.ini @@ -95,6 +95,7 @@ flake8-ignore = scrapy/loader/__init__.py E501 E128 scrapy/loader/processors.py E501 # scrapy/pipelines + scrapy/pipelines/__init__.py E501 scrapy/pipelines/files.py E116 E501 E266 scrapy/pipelines/images.py E265 E501 scrapy/pipelines/media.py E125 E501 E266 diff --git a/scrapy/pipelines/__init__.py b/scrapy/pipelines/__init__.py index 1a45e00a2..b5725a8ee 100644 --- a/scrapy/pipelines/__init__.py +++ b/scrapy/pipelines/__init__.py @@ -9,7 +9,6 @@ from scrapy.utils.conf import build_component_list from scrapy.utils.defer import deferred_f_from_coro_f - class ItemPipelineManager(MiddlewareManager): component_name = 'item pipeline' From 7d859848800ef05760e4f57dc206a4d756460a9d Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 9 Jan 2020 14:48:07 +0500 Subject: [PATCH 51/81] Use get_from_asyncio_queue in the pipeline test. --- tests/test_pipelines.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index 6f33282f2..c72f1a338 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -6,7 +6,7 @@ from twisted.internet.defer import Deferred from twisted.trial import unittest from scrapy import Spider, signals, Request -from scrapy.utils.test import get_crawler +from scrapy.utils.test import get_crawler, get_from_asyncio_queue from tests.mockserver import MockServer @@ -39,7 +39,7 @@ class AsyncDefPipeline: class AsyncDefAsyncioPipeline: async def process_item(self, item, spider): await asyncio.sleep(0.2) - item['pipeline_passed'] = True + item['pipeline_passed'] = await get_from_asyncio_queue(True) return item From 735c0ceb7890dbea607dd6e4c0a14a4ce2b0afd2 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 9 Sep 2019 16:20:58 -0300 Subject: [PATCH 52/81] Custom name resolver implementing twisted.internet.interfaces.IHostnameResolver --- pytest.ini | 1 + scrapy/crawler.py | 38 ++++++++++++++++++---------------- scrapy/resolver.py | 51 ++++++++++++++++++++++++++-------------------- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/pytest.ini b/pytest.ini index c3f3292bb..a0e89f0a9 100644 --- a/pytest.ini +++ b/pytest.ini @@ -158,6 +158,7 @@ flake8-ignore = scrapy/mail.py E402 E128 E501 E502 scrapy/middleware.py E128 E501 scrapy/pqueues.py E501 + scrapy/resolver.py E501 scrapy/responsetypes.py E128 E501 E305 scrapy/robotstxt.py E501 scrapy/shell.py E501 diff --git a/scrapy/crawler.py b/scrapy/crawler.py index f87e67d93..1350ea84f 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -4,34 +4,36 @@ import signal import warnings from twisted.internet import defer -from zope.interface.verify import verifyClass, DoesNotImplement +from zope.interface.verify import DoesNotImplement, verifyClass -from scrapy import Spider +from scrapy import signals, Spider from scrapy.core.engine import ExecutionEngine -from scrapy.resolver import CachingThreadedResolver -from scrapy.interfaces import ISpiderLoader +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.extension import ExtensionManager +from scrapy.interfaces import ISpiderLoader +from scrapy.resolver import CachingHostnameResolver from scrapy.settings import overridden_settings, Settings from scrapy.signalmanager import SignalManager -from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.asyncio import install_asyncio_reactor, is_asyncio_reactor_installed -from scrapy.utils.ossignal import install_shutdown_handlers, signal_names -from scrapy.utils.misc import load_object from scrapy.utils.log import ( - LogCounterHandler, configure_logging, log_scrapy_info, - get_scrapy_root_handler, install_scrapy_root_handler) -from scrapy import signals + configure_logging, + get_scrapy_root_handler, + install_scrapy_root_handler, + log_scrapy_info, + LogCounterHandler, +) +from scrapy.utils.misc import load_object +from scrapy.utils.ossignal import install_shutdown_handlers, signal_names logger = logging.getLogger(__name__) -class Crawler(object): +class Crawler: def __init__(self, spidercls, settings=None): if isinstance(spidercls, Spider): - raise ValueError( - 'The spidercls argument must be a class, not an object') + raise ValueError('The spidercls argument must be a class, not an object') if isinstance(settings, dict) or settings is None: settings = Settings(settings) @@ -110,7 +112,7 @@ class Crawler(object): yield defer.maybeDeferred(self.engine.stop) -class CrawlerRunner(object): +class CrawlerRunner: """ This is a convenient helper class that keeps track of, manages and runs crawlers inside an already setup :mod:`~twisted.internet.reactor`. @@ -303,7 +305,7 @@ class CrawlerProcess(CrawlerRunner): return d.addBoth(self._stop_reactor) - reactor.installResolver(self._get_dns_resolver()) + reactor.installNameResolver(self._get_dns_resolver()) tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) @@ -315,10 +317,10 @@ class CrawlerProcess(CrawlerRunner): cache_size = self.settings.getint('DNSCACHE_SIZE') else: cache_size = 0 - return CachingThreadedResolver( - reactor=reactor, + return CachingHostnameResolver( + resolver=reactor.nameResolver, cache_size=cache_size, - timeout=self.settings.getfloat('DNS_TIMEOUT') + timeout=self.settings.getfloat('DNS_TIMEOUT'), ) def _graceful_stop_reactor(self): diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 4df949015..03964f269 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -1,32 +1,39 @@ -from twisted.internet import defer -from twisted.internet.base import ThreadedResolver +from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver +from zope.interface.declarations import implementer, provider from scrapy.utils.datatypes import LocalCache -# TODO: cache misses +# TODO: cache misses dnscache = LocalCache(10000) -class CachingThreadedResolver(ThreadedResolver): - def __init__(self, reactor, cache_size, timeout): - super(CachingThreadedResolver, self).__init__(reactor) - dnscache.limit = cache_size +@implementer(IHostnameResolver) +class CachingHostnameResolver(object): + + def __init__(self, resolver, cache_size, timeout): + self.resolver = resolver self.timeout = timeout + dnscache.limit = cache_size - def getHostByName(self, name, timeout=None): - if name in dnscache: - return defer.succeed(dnscache[name]) - # in Twisted<=16.6, getHostByName() is always called with - # a default timeout of 60s (actually passed as (1, 3, 11, 45) tuple), - # so the input argument above is simply overridden - # to enforce Scrapy's DNS_TIMEOUT setting's value - timeout = (self.timeout,) - d = super(CachingThreadedResolver, self).getHostByName(name, timeout) - if dnscache.limit: - d.addCallback(self._cache_result, name) - return d + def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, + addressTypes=None, transportSemantics='TCP'): - def _cache_result(self, result, name): - dnscache[name] = result - return result + @provider(IResolutionReceiver) + class CachingResolutionReceiver(resolutionReceiver): + def resolutionBegan(self, resolution): + super(CachingResolutionReceiver, self).resolutionBegan(resolution) + self.resolution = resolution + + def resolutionComplete(self): + super(CachingResolutionReceiver, self).resolutionComplete() + dnscache[hostName] = self.resolution + + try: + result = dnscache[hostName] + except KeyError: + result = self.resolver.resolveHostName( + CachingResolutionReceiver(), hostName, portNumber, addressTypes, transportSemantics + ) + finally: + return result From f1c184631e8cefc191dc0077083138c241bfd6a8 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 11 Dec 2019 17:44:05 -0300 Subject: [PATCH 53/81] Name resolver: timeout --- scrapy/resolver.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 03964f269..8792ed6ab 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -1,3 +1,4 @@ +from twisted.internet import reactor from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver from zope.interface.declarations import implementer, provider @@ -21,9 +22,14 @@ class CachingHostnameResolver(object): @provider(IResolutionReceiver) class CachingResolutionReceiver(resolutionReceiver): + + def __init__(self, timeout): + self.timeout = timeout + def resolutionBegan(self, resolution): super(CachingResolutionReceiver, self).resolutionBegan(resolution) self.resolution = resolution + # reactor.callLater(self.timeout, resolution.cancel) def resolutionComplete(self): super(CachingResolutionReceiver, self).resolutionComplete() @@ -33,7 +39,11 @@ class CachingHostnameResolver(object): result = dnscache[hostName] except KeyError: result = self.resolver.resolveHostName( - CachingResolutionReceiver(), hostName, portNumber, addressTypes, transportSemantics + CachingResolutionReceiver(self.timeout), + hostName, + portNumber, + addressTypes, + transportSemantics ) finally: return result From 55babf9acd6cd357f4211a86af01a1f2abe2f0cb Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 15 Jan 2020 12:25:20 -0300 Subject: [PATCH 54/81] Cache resolution only if the DNS request was successful --- scrapy/resolver.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 8792ed6ab..0ba22ed0d 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -10,7 +10,7 @@ dnscache = LocalCache(10000) @implementer(IHostnameResolver) -class CachingHostnameResolver(object): +class CachingHostnameResolver: def __init__(self, resolver, cache_size, timeout): self.resolver = resolver @@ -25,15 +25,21 @@ class CachingHostnameResolver(object): def __init__(self, timeout): self.timeout = timeout + self.resolved = False def resolutionBegan(self, resolution): super(CachingResolutionReceiver, self).resolutionBegan(resolution) self.resolution = resolution # reactor.callLater(self.timeout, resolution.cancel) + def addressResolved(self, address): + super(CachingResolutionReceiver, self).addressResolved(address) + self.resolved = True + def resolutionComplete(self): super(CachingResolutionReceiver, self).resolutionComplete() - dnscache[hostName] = self.resolution + if self.resolved: + dnscache[hostName] = self.resolution try: result = dnscache[hostName] From 8c3de288fa2564473f49198b40efdeaa2428f1ca Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 15 Jan 2020 12:31:36 -0300 Subject: [PATCH 55/81] Remove non-working DNS timeout code --- scrapy/crawler.py | 1 - scrapy/resolver.py | 12 +++--------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 1350ea84f..61851acc3 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -320,7 +320,6 @@ class CrawlerProcess(CrawlerRunner): return CachingHostnameResolver( resolver=reactor.nameResolver, cache_size=cache_size, - timeout=self.settings.getfloat('DNS_TIMEOUT'), ) def _graceful_stop_reactor(self): diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 0ba22ed0d..ddbae61a9 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -1,4 +1,3 @@ -from twisted.internet import reactor from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver from zope.interface.declarations import implementer, provider @@ -12,9 +11,8 @@ dnscache = LocalCache(10000) @implementer(IHostnameResolver) class CachingHostnameResolver: - def __init__(self, resolver, cache_size, timeout): + def __init__(self, resolver, cache_size): self.resolver = resolver - self.timeout = timeout dnscache.limit = cache_size def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, @@ -23,14 +21,10 @@ class CachingHostnameResolver: @provider(IResolutionReceiver) class CachingResolutionReceiver(resolutionReceiver): - def __init__(self, timeout): - self.timeout = timeout - self.resolved = False - def resolutionBegan(self, resolution): super(CachingResolutionReceiver, self).resolutionBegan(resolution) self.resolution = resolution - # reactor.callLater(self.timeout, resolution.cancel) + self.resolved = False def addressResolved(self, address): super(CachingResolutionReceiver, self).addressResolved(address) @@ -45,7 +39,7 @@ class CachingHostnameResolver: result = dnscache[hostName] except KeyError: result = self.resolver.resolveHostName( - CachingResolutionReceiver(self.timeout), + CachingResolutionReceiver(), hostName, portNumber, addressTypes, From e69cf415c8326349c871fb23faba2ed822aa08ee Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 03:58:07 -0300 Subject: [PATCH 56/81] Ability to choose name resolver --- scrapy/crawler.py | 14 ++------ scrapy/resolver.py | 54 ++++++++++++++++++++++++++++- scrapy/settings/default_settings.py | 1 + 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 61851acc3..c5351c08f 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -305,23 +305,13 @@ class CrawlerProcess(CrawlerRunner): return d.addBoth(self._stop_reactor) - reactor.installNameResolver(self._get_dns_resolver()) + resolver_class = load_object(self.settings["DNS_RESOLVER"]) + resolver_class.install(reactor, self.settings) tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) reactor.run(installSignalHandlers=False) # blocking call - def _get_dns_resolver(self): - from twisted.internet import reactor - if self.settings.getbool('DNSCACHE_ENABLED'): - cache_size = self.settings.getint('DNSCACHE_SIZE') - else: - cache_size = 0 - return CachingHostnameResolver( - resolver=reactor.nameResolver, - cache_size=cache_size, - ) - def _graceful_stop_reactor(self): d = self.stop() d.addBoth(self._stop_reactor) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index ddbae61a9..2bef9f1b8 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -1,4 +1,6 @@ -from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver +from twisted.internet import defer +from twisted.internet.base import ThreadedResolver +from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver, IResolverSimple from zope.interface.declarations import implementer, provider from scrapy.utils.datatypes import LocalCache @@ -8,8 +10,58 @@ from scrapy.utils.datatypes import LocalCache dnscache = LocalCache(10000) +@implementer(IResolverSimple) +class CachingThreadedResolver(ThreadedResolver): + """ + Default caching resolver. IPv4 only, supports setting a timeout value for DNS requests + """ + + @classmethod + def install(cls, reactor, settings): + if settings.getbool('DNSCACHE_ENABLED'): + cache_size = settings.getint('DNSCACHE_SIZE') + else: + cache_size = 0 + resolver = cls(reactor, cache_size, settings.getfloat('DNS_TIMEOUT')) + reactor.installResolver(resolver) + + def __init__(self, reactor, cache_size, timeout): + super(CachingThreadedResolver, self).__init__(reactor) + dnscache.limit = cache_size + self.timeout = timeout + + def getHostByName(self, name, timeout=None): + if name in dnscache: + return defer.succeed(dnscache[name]) + # in Twisted<=16.6, getHostByName() is always called with + # a default timeout of 60s (actually passed as (1, 3, 11, 45) tuple), + # so the input argument above is simply overridden + # to enforce Scrapy's DNS_TIMEOUT setting's value + timeout = (self.timeout,) + d = super(CachingThreadedResolver, self).getHostByName(name, timeout) + if dnscache.limit: + d.addCallback(self._cache_result, name) + return d + + def _cache_result(self, result, name): + dnscache[name] = result + return result + + @implementer(IHostnameResolver) class CachingHostnameResolver: + """ + Experimental caching resolver, supporting IPv4 and IPv6 + """ + + @classmethod + def install(cls, reactor, settings): + if settings.getbool('DNSCACHE_ENABLED'): + cache_size = settings.getint('DNSCACHE_SIZE') + else: + cache_size = 0 + resolver = cls(reactor.nameResolver, cache_size) + reactor.installNameResolver(resolver) def __init__(self, resolver, cache_size): self.resolver = resolver diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index d03fd37b0..46ed3be96 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -60,6 +60,7 @@ DEPTH_PRIORITY = 0 DNSCACHE_ENABLED = True DNSCACHE_SIZE = 10000 +DNS_RESOLVER = 'scrapy.resolver.CachingThreadedResolver' DNS_TIMEOUT = 60 DOWNLOAD_DELAY = 0 From 0f155b059a43b2cc48149a26c6584910038a23f1 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 04:27:13 -0300 Subject: [PATCH 57/81] Make Flake8 happy (remove unused import) --- scrapy/crawler.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index c5351c08f..5658e264b 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -11,7 +11,6 @@ from scrapy.core.engine import ExecutionEngine from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.extension import ExtensionManager from scrapy.interfaces import ISpiderLoader -from scrapy.resolver import CachingHostnameResolver from scrapy.settings import overridden_settings, Settings from scrapy.signalmanager import SignalManager from scrapy.utils.asyncio import install_asyncio_reactor, is_asyncio_reactor_installed From f45b4c7f8d191c6855c12f3497ece6b6121289df Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 10:09:34 -0300 Subject: [PATCH 58/81] from_crawler support for name resolvers --- scrapy/crawler.py | 2 +- scrapy/resolver.py | 48 ++++++++++++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 5658e264b..4531c3aac 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -305,7 +305,7 @@ class CrawlerProcess(CrawlerRunner): d.addBoth(self._stop_reactor) resolver_class = load_object(self.settings["DNS_RESOLVER"]) - resolver_class.install(reactor, self.settings) + resolver_class.install_on_reactor(reactor, crawler=self) tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 2bef9f1b8..792563e79 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -4,6 +4,7 @@ from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver, from zope.interface.declarations import implementer, provider from scrapy.utils.datatypes import LocalCache +from scrapy.utils.misc import create_instance # TODO: cache misses @@ -13,23 +14,27 @@ dnscache = LocalCache(10000) @implementer(IResolverSimple) class CachingThreadedResolver(ThreadedResolver): """ - Default caching resolver. IPv4 only, supports setting a timeout value for DNS requests + Default caching resolver. IPv4 only, supports setting a timeout value for DNS requests. """ - @classmethod - def install(cls, reactor, settings): - if settings.getbool('DNSCACHE_ENABLED'): - cache_size = settings.getint('DNSCACHE_SIZE') - else: - cache_size = 0 - resolver = cls(reactor, cache_size, settings.getfloat('DNS_TIMEOUT')) - reactor.installResolver(resolver) - def __init__(self, reactor, cache_size, timeout): super(CachingThreadedResolver, self).__init__(reactor) dnscache.limit = cache_size self.timeout = timeout + @classmethod + def from_crawler(cls, crawler, reactor): + if crawler.settings.getbool('DNSCACHE_ENABLED'): + cache_size = crawler.settings.getint('DNSCACHE_SIZE') + else: + cache_size = 0 + return cls(reactor, cache_size, crawler.settings.getfloat('DNS_TIMEOUT')) + + @classmethod + def install_on_reactor(cls, reactor, crawler): + resolver = create_instance(cls, None, crawler, reactor=reactor) + reactor.installResolver(resolver) + def getHostByName(self, name, timeout=None): if name in dnscache: return defer.succeed(dnscache[name]) @@ -51,21 +56,26 @@ class CachingThreadedResolver(ThreadedResolver): @implementer(IHostnameResolver) class CachingHostnameResolver: """ - Experimental caching resolver, supporting IPv4 and IPv6 + Experimental caching resolver. Resolves IPv4 and IPv6 addresses, + does not support setting a timeout value for DNS requests. """ + def __init__(self, reactor, cache_size): + self.resolver = reactor.nameResolver + dnscache.limit = cache_size + @classmethod - def install(cls, reactor, settings): - if settings.getbool('DNSCACHE_ENABLED'): - cache_size = settings.getint('DNSCACHE_SIZE') + def from_crawler(cls, crawler, reactor): + if crawler.settings.getbool('DNSCACHE_ENABLED'): + cache_size = crawler.settings.getint('DNSCACHE_SIZE') else: cache_size = 0 - resolver = cls(reactor.nameResolver, cache_size) - reactor.installNameResolver(resolver) + return cls(reactor, cache_size) - def __init__(self, resolver, cache_size): - self.resolver = resolver - dnscache.limit = cache_size + @classmethod + def install_on_reactor(cls, reactor, crawler): + resolver = create_instance(cls, None, crawler, reactor=reactor) + reactor.installNameResolver(resolver) def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, addressTypes=None, transportSemantics='TCP'): From 3cfa73b8b12dfe2dd1364af856d2ec46a486b48d Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 18:01:18 -0300 Subject: [PATCH 59/81] Name resolvers: install_on_reactor as instance method --- scrapy/crawler.py | 5 +++-- scrapy/resolver.py | 13 ++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 4531c3aac..0cef75a6a 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -21,7 +21,7 @@ from scrapy.utils.log import ( log_scrapy_info, LogCounterHandler, ) -from scrapy.utils.misc import load_object +from scrapy.utils.misc import create_instance, load_object from scrapy.utils.ossignal import install_shutdown_handlers, signal_names @@ -305,7 +305,8 @@ class CrawlerProcess(CrawlerRunner): d.addBoth(self._stop_reactor) resolver_class = load_object(self.settings["DNS_RESOLVER"]) - resolver_class.install_on_reactor(reactor, crawler=self) + resolver = create_instance(resolver_class, self.settings, self, reactor=reactor) + resolver.install_on_reactor(reactor) tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 792563e79..2b97603da 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -4,7 +4,6 @@ from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver, from zope.interface.declarations import implementer, provider from scrapy.utils.datatypes import LocalCache -from scrapy.utils.misc import create_instance # TODO: cache misses @@ -30,10 +29,8 @@ class CachingThreadedResolver(ThreadedResolver): cache_size = 0 return cls(reactor, cache_size, crawler.settings.getfloat('DNS_TIMEOUT')) - @classmethod - def install_on_reactor(cls, reactor, crawler): - resolver = create_instance(cls, None, crawler, reactor=reactor) - reactor.installResolver(resolver) + def install_on_reactor(self, reactor): + reactor.installResolver(self) def getHostByName(self, name, timeout=None): if name in dnscache: @@ -72,10 +69,8 @@ class CachingHostnameResolver: cache_size = 0 return cls(reactor, cache_size) - @classmethod - def install_on_reactor(cls, reactor, crawler): - resolver = create_instance(cls, None, crawler, reactor=reactor) - reactor.installNameResolver(resolver) + def install_on_reactor(self, reactor): + reactor.installNameResolver(self) def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, addressTypes=None, transportSemantics='TCP'): From 1040f581ec1ba3bcf8f08edb824d59c0e4a93700 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 20:14:52 -0300 Subject: [PATCH 60/81] Name resolvers: do not pass the reactor to the install method --- scrapy/crawler.py | 2 +- scrapy/resolver.py | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 0cef75a6a..35c6b7716 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -306,7 +306,7 @@ class CrawlerProcess(CrawlerRunner): resolver_class = load_object(self.settings["DNS_RESOLVER"]) resolver = create_instance(resolver_class, self.settings, self, reactor=reactor) - resolver.install_on_reactor(reactor) + resolver.install_on_reactor() tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 2b97603da..7c776f75e 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -19,6 +19,7 @@ class CachingThreadedResolver(ThreadedResolver): def __init__(self, reactor, cache_size, timeout): super(CachingThreadedResolver, self).__init__(reactor) dnscache.limit = cache_size + self.reactor = reactor self.timeout = timeout @classmethod @@ -29,8 +30,8 @@ class CachingThreadedResolver(ThreadedResolver): cache_size = 0 return cls(reactor, cache_size, crawler.settings.getfloat('DNS_TIMEOUT')) - def install_on_reactor(self, reactor): - reactor.installResolver(self) + def install_on_reactor(self,): + self.reactor.installResolver(self) def getHostByName(self, name, timeout=None): if name in dnscache: @@ -58,7 +59,8 @@ class CachingHostnameResolver: """ def __init__(self, reactor, cache_size): - self.resolver = reactor.nameResolver + self.reactor = reactor + self.original_resolver = reactor.nameResolver dnscache.limit = cache_size @classmethod @@ -69,8 +71,8 @@ class CachingHostnameResolver: cache_size = 0 return cls(reactor, cache_size) - def install_on_reactor(self, reactor): - reactor.installNameResolver(self) + def install_on_reactor(self): + self.reactor.installNameResolver(self) def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, addressTypes=None, transportSemantics='TCP'): @@ -95,7 +97,7 @@ class CachingHostnameResolver: try: result = dnscache[hostName] except KeyError: - result = self.resolver.resolveHostName( + result = self.original_resolver.resolveHostName( CachingResolutionReceiver(), hostName, portNumber, From 90e3bd8715701aeea9187837bc32925e3225288f Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 20:32:40 -0300 Subject: [PATCH 61/81] [test] Name resolvers --- tests/CrawlerProcess/alternative_name_resolver.py | 15 +++++++++++++++ tests/CrawlerProcess/default_name_resolver.py | 12 ++++++++++++ tests/test_crawler.py | 12 ++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 tests/CrawlerProcess/alternative_name_resolver.py create mode 100644 tests/CrawlerProcess/default_name_resolver.py diff --git a/tests/CrawlerProcess/alternative_name_resolver.py b/tests/CrawlerProcess/alternative_name_resolver.py new file mode 100644 index 000000000..2c466da04 --- /dev/null +++ b/tests/CrawlerProcess/alternative_name_resolver.py @@ -0,0 +1,15 @@ +import scrapy +from scrapy.crawler import CrawlerProcess + + +class IPv6Spider(scrapy.Spider): + name = "ipv6_spider" + start_urls = ["http://[::1]"] + + +process = CrawlerProcess(settings={ + "RETRY_ENABLED": False, + "DNS_RESOLVER": "scrapy.resolver.CachingHostnameResolver", +}) +process.crawl(IPv6Spider) +process.start() diff --git a/tests/CrawlerProcess/default_name_resolver.py b/tests/CrawlerProcess/default_name_resolver.py new file mode 100644 index 000000000..60d91b68b --- /dev/null +++ b/tests/CrawlerProcess/default_name_resolver.py @@ -0,0 +1,12 @@ +import scrapy +from scrapy.crawler import CrawlerProcess + + +class IPv6Spider(scrapy.Spider): + name = "ipv6_spider" + start_urls = ["http://[::1]"] + + +process = CrawlerProcess(settings={"RETRY_ENABLED": False}) +process.crawl(IPv6Spider) +process.start() diff --git a/tests/test_crawler.py b/tests/test_crawler.py index fce60ca37..d85f2bc41 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -305,3 +305,15 @@ class CrawlerProcessSubprocess(unittest.TestCase): log = self.run_script('asyncio_enabled_reactor.py') self.assertIn('Spider closed (finished)', log) self.assertIn("DEBUG: Asyncio reactor is installed", log) + + def test_default_name_resolver(self): + log = self.run_script('default_name_resolver.py') + self.assertIn('Spider closed (finished)', log) + self.assertIn("twisted.internet.error.DNSLookupError: DNS lookup failed: no results for hostname lookup: ::1.", log) + self.assertIn("'downloader/exception_type_count/twisted.internet.error.DNSLookupError': 1,", log) + + def test_alternative_name_resolver(self): + log = self.run_script('alternative_name_resolver.py') + self.assertIn('Spider closed (finished)', log) + self.assertIn("twisted.internet.error.ConnectionRefusedError: Connection was refused by other side: 111: Connection refused.", log) + self.assertIn("'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1,", log) From d487498cff893d2c507ef5ed0957f101dc3ab4a9 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 22:02:01 -0300 Subject: [PATCH 62/81] Update name resolvers tests --- tests/test_crawler.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index d85f2bc41..14f4f8418 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -306,14 +306,20 @@ class CrawlerProcessSubprocess(unittest.TestCase): self.assertIn('Spider closed (finished)', log) self.assertIn("DEBUG: Asyncio reactor is installed", log) - def test_default_name_resolver(self): + def test_ipv6_default_name_resolver(self): log = self.run_script('default_name_resolver.py') self.assertIn('Spider closed (finished)', log) self.assertIn("twisted.internet.error.DNSLookupError: DNS lookup failed: no results for hostname lookup: ::1.", log) self.assertIn("'downloader/exception_type_count/twisted.internet.error.DNSLookupError': 1,", log) - def test_alternative_name_resolver(self): + def test_ipv6_alternative_name_resolver(self): log = self.run_script('alternative_name_resolver.py') self.assertIn('Spider closed (finished)', log) - self.assertIn("twisted.internet.error.ConnectionRefusedError: Connection was refused by other side: 111: Connection refused.", log) - self.assertIn("'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1,", log) + self.assertTrue(any( + "twisted.internet.error.ConnectionRefusedError" in log, + "twisted.internet.error.ConnectError" in log, + )) + self.assertTrue(any( + "'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1," in log, + "'downloader/exception_type_count/twisted.internet.error.ConnectError': 1," in log, + )) From dee420a69cb5c8319a7df99a5765c5ca90a7063e Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 23:48:16 -0300 Subject: [PATCH 63/81] Fix name resolvers tests --- tests/test_crawler.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 14f4f8418..0ce0674de 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -315,11 +315,11 @@ class CrawlerProcessSubprocess(unittest.TestCase): def test_ipv6_alternative_name_resolver(self): log = self.run_script('alternative_name_resolver.py') self.assertIn('Spider closed (finished)', log) - self.assertTrue(any( + self.assertTrue(any([ "twisted.internet.error.ConnectionRefusedError" in log, "twisted.internet.error.ConnectError" in log, - )) - self.assertTrue(any( + ])) + self.assertTrue(any([ "'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1," in log, "'downloader/exception_type_count/twisted.internet.error.ConnectError': 1," in log, - )) + ])) From 41f7ebf3add25b2251082b206b062c00d1daba58 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 17 Jan 2020 12:40:49 -0300 Subject: [PATCH 64/81] CachingThreadedResolver: No need to store the reactor as an instance attribute It's already done in the parent class --- scrapy/resolver.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 7c776f75e..7751f3796 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -19,7 +19,6 @@ class CachingThreadedResolver(ThreadedResolver): def __init__(self, reactor, cache_size, timeout): super(CachingThreadedResolver, self).__init__(reactor) dnscache.limit = cache_size - self.reactor = reactor self.timeout = timeout @classmethod From 302d3f552b5a930a43fddac8eb8c5f769d8748bc Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 18 Jan 2020 01:41:57 -0300 Subject: [PATCH 65/81] [doc] DNS_RESOLVER setting --- docs/topics/settings.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index c02f877fc..292eaea74 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -397,6 +397,19 @@ Default: ``10000`` DNS in-memory cache size. +.. setting:: DNS_RESOLVER + +DNS_RESOLVER +------------ + +Default: ``'scrapy.resolver.CachingThreadedResolver'`` + +The class to be used to resolve DNS names. The default ``scrapy.resolver.CachingThreadedResolver`` +supports specifying a timeout for DNS requests via the :setting:`DNS_TIMEOUT` setting, +but works only with IPv4 addresses. Scrapy provides an alternative resolver, +``scrapy.resolver.CachingHostnameResolver``, which supports IPv4/IPv6 addresses but does not +take the :setting:`DNS_TIMEOUT` setting into account. + .. setting:: DNS_TIMEOUT DNS_TIMEOUT From b471765d40a2a8a94f90af46ac6903e3786aec5f Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 18 Jan 2020 01:52:29 -0300 Subject: [PATCH 66/81] [doc] FAQ entry about the IPv6 and the DNS_RESOLVER setting --- docs/faq.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/faq.rst b/docs/faq.rst index aae2411e0..b789a8cdb 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -353,6 +353,13 @@ method for this purpose. For example:: for _ in range(item['multiply_by']): yield deepcopy(item) +Does Scrapy support IPv6 addresses? +----------------------------------- + +Yes, by setting :setting:`DNS_RESOLVER` to ``scrapy.resolver.CachingHostnameResolver``. +Note that by doing so, you lose the ability to set a specific timeout for DNS requests +(the value of the :setting:`DNS_TIMEOUT` setting is ignored). + .. _user agents: https://en.wikipedia.org/wiki/User_agent .. _LIFO: https://en.wikipedia.org/wiki/Stack_(abstract_data_type) From eaa8ed02d05d0cc5e7be1f773a22755676c7fe56 Mon Sep 17 00:00:00 2001 From: Juan Pablo Balarini Date: Wed, 26 Dec 2018 13:11:23 -0300 Subject: [PATCH 67/81] Add ability to change max_active_size by settings --- docs/topics/settings.rst | 11 +++++++++++ scrapy/core/scraper.py | 2 +- scrapy/settings/default_settings.py | 2 ++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index c02f877fc..d4480c879 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1262,6 +1262,17 @@ Type of priority queue used by the scheduler. Another available type is domains in parallel. But currently ``scrapy.pqueues.DownloaderAwarePriorityQueue`` does not work together with :setting:`CONCURRENT_REQUESTS_PER_IP`. +.. setting:: SCRAPER_SLOT_MAX_ACTIVE_SIZE + +SCRAPER_SLOT_MAX_ACTIVE_SIZE +---------------------------- +Default: ``5000000`` + +Soft limit (in bytes) for response data being processed. + +While the sum of the sizes of all responses being processed is above this value, +Scrapy does not process new requests. + .. setting:: SPIDER_CONTRACTS SPIDER_CONTRACTS diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 99114d3bb..facbd8b73 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -78,7 +78,7 @@ class Scraper(object): @defer.inlineCallbacks def open_spider(self, spider): """Open the given spider for scraping and allocate resources for it""" - self.slot = Slot() + self.slot = Slot(self.crawler.settings.getint('SCRAPER_SLOT_MAX_ACTIVE_SIZE')) yield self.itemproc.open_spider(spider) def close_spider(self, spider): diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index d03fd37b0..ddd48d327 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -254,6 +254,8 @@ SCHEDULER_DISK_QUEUE = 'scrapy.squeues.PickleLifoDiskQueue' SCHEDULER_MEMORY_QUEUE = 'scrapy.squeues.LifoMemoryQueue' SCHEDULER_PRIORITY_QUEUE = 'scrapy.pqueues.ScrapyPriorityQueue' +SCRAPER_SLOT_MAX_ACTIVE_SIZE = 5000000 + SPIDER_LOADER_CLASS = 'scrapy.spiderloader.SpiderLoader' SPIDER_LOADER_WARN_ONLY = False From 0f2d871d88b6741aaf0348d1f11a1e020124099f Mon Sep 17 00:00:00 2001 From: JP Balarini Date: Wed, 22 May 2019 16:26:27 -0300 Subject: [PATCH 68/81] Use PEP 515 style for SCRAPER_SLOT_MAX_ACTIVE_SIZE documentation --- docs/topics/settings.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index d4480c879..f4c3494f5 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1266,7 +1266,7 @@ does not work together with :setting:`CONCURRENT_REQUESTS_PER_IP`. SCRAPER_SLOT_MAX_ACTIVE_SIZE ---------------------------- -Default: ``5000000`` +Default: ``5_000_000`` Soft limit (in bytes) for response data being processed. From 8ea8f14827470f37c0e53d302aa65bcfa9604f3c Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Mon, 20 Jan 2020 18:19:36 +0200 Subject: [PATCH 69/81] Update scrapy/utils/ftp.py Co-Authored-By: Mikhail Korobov --- scrapy/utils/ftp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index bf67b9976..b3e9ec2ed 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -17,7 +17,7 @@ def ftp_makedirs_cwd(ftp, path, first_call=True): ftp.cwd(path) def ftp_store_file( - path, file, host ,port, + path, file, host, port, username, password, use_active_mode=False): """Opens a FTP connection with passed credentials,sets current directory to the directory extracted from given path, then uploads the file to server From 06ab668ec7f880f9992dc669a374a6111cef5d04 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Wed, 22 Jan 2020 03:48:07 +0200 Subject: [PATCH 70/81] Use kwargs-only parameters in `ftp_store_file` --- scrapy/extensions/feedexport.py | 6 +++--- scrapy/pipelines/files.py | 6 +++--- scrapy/utils/ftp.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 1ddc55f93..06b5a0dd9 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -175,9 +175,9 @@ class FTPFeedStorage(BlockingFeedStorage): def _store_in_thread(self, file): ftp_store_file( - self.path, file, self.host, - self.port, self.username, - self.password, self.use_active_mode + path=self.path, file=file, host=self.host, + port=self.port, username=self.username, + password=self.password, use_active_mode=self.use_active_mode ) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 5780f63bd..5383b05fe 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -269,9 +269,9 @@ class FTPFilesStore(object): def persist_file(self, path, buf, info, meta=None, headers=None): path = '%s/%s' % (self.basedir, path) return threads.deferToThread( - ftp_store_file, path,buf, - self.host, self.port,self.username, - self.password, self.USE_ACTIVE_MODE + ftp_store_file, path=path, file=buf, + host=self.host, port=self.port, username=self.username, + password=self.password, use_active_mode=self.USE_ACTIVE_MODE ) def stat_file(self, path, info): diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index b3e9ec2ed..752e3c953 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -17,7 +17,7 @@ def ftp_makedirs_cwd(ftp, path, first_call=True): ftp.cwd(path) def ftp_store_file( - path, file, host, port, + *, path, file, host, port, username, password, use_active_mode=False): """Opens a FTP connection with passed credentials,sets current directory to the directory extracted from given path, then uploads the file to server From c75cf15b7a8293fd83f6e8b27abca2e50b0a04ed Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 22 Jan 2020 10:38:59 -0300 Subject: [PATCH 71/81] Update CSS selectors in tutorial --- docs/intro/tutorial.rst | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index c4d8d717b..c9d00eb74 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -616,19 +616,24 @@ instance; you still have to yield this Request. You can also pass a selector to ``response.follow`` instead of a string; this selector should extract necessary attributes:: - href = response.css('li.next a::attr(href)')[0] - yield response.follow(href, callback=self.parse) + for href in response.css('ul.pager a::attr(href)'): + yield response.follow(href, callback=self.parse) For ```` elements there is a shortcut: ``response.follow`` uses their href attribute automatically. So the code can be shortened further:: - a = response.css('li.next a')[0] - yield response.follow(a, callback=self.parse) + for a in response.css('ul.pager a'): + yield response.follow(a, callback=self.parse) To create multiple requests from an iterable, you can use :meth:`response.follow_all ` instead:: - yield from response.follow_all(response.css('a'), callback=self.parse) + anchors = response.css('ul.pager a') + yield from response.follow_all(anchors, callback=self.parse) + +or, shortening it further:: + + yield from response.follow_all(css='ul.pager a', callback=self.parse) More examples and patterns From 7d5cebcf773d54d2f7b5ac0b90886847a4a70c26 Mon Sep 17 00:00:00 2001 From: Peter Vandenabeele Date: Thu, 23 Jan 2020 09:08:21 +0100 Subject: [PATCH 72/81] fix logical documentation error with PER_DOMAIN or PER_DOMAIN --- docs/faq.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/faq.rst b/docs/faq.rst index aae2411e0..169e1a479 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -140,7 +140,7 @@ setting the following settings:: While pending requests are below the configured values of :setting:`CONCURRENT_REQUESTS`, :setting:`CONCURRENT_REQUESTS_PER_DOMAIN` or -:setting:`CONCURRENT_REQUESTS_PER_DOMAIN`, those requests are sent +:setting:`CONCURRENT_REQUESTS_PER_IP`, those requests are sent concurrently. As a result, the first few requests of a crawl rarely follow the desired order. Lowering those settings to ``1`` enforces the desired order, but it significantly slows down the crawl as a whole. From 9899414300b4a6491ea17418c3403aed08e0faf6 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 23 Jan 2020 18:06:59 -0300 Subject: [PATCH 73/81] Name resolver: return result directly --- scrapy/resolver.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 7751f3796..554a3a14d 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -94,14 +94,12 @@ class CachingHostnameResolver: dnscache[hostName] = self.resolution try: - result = dnscache[hostName] + return dnscache[hostName] except KeyError: - result = self.original_resolver.resolveHostName( + return self.original_resolver.resolveHostName( CachingResolutionReceiver(), hostName, portNumber, addressTypes, transportSemantics ) - finally: - return result From c544c0d2b8356125d1a5465b44617aaaaeab0ea1 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Fri, 24 Jan 2020 14:36:16 +0200 Subject: [PATCH 74/81] Use context management with `FTP` --- scrapy/utils/ftp.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index 752e3c953..9992a916e 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -22,13 +22,12 @@ def ftp_store_file( """Opens a FTP connection with passed credentials,sets current directory to the directory extracted from given path, then uploads the file to server """ - ftp = FTP() - ftp.connect(host, port) - ftp.login(username, password) - if use_active_mode: - ftp.set_pasv(False) - file.seek(0) - dirname, filename = posixpath.split(path) - ftp_makedirs_cwd(ftp, dirname) - ftp.storbinary('STOR %s' % filename, file) - ftp.quit() + with FTP() as ftp: + ftp.connect(host, port) + ftp.login(username, password) + if use_active_mode: + ftp.set_pasv(False) + file.seek(0) + dirname, filename = posixpath.split(path) + ftp_makedirs_cwd(ftp, dirname) + ftp.storbinary('STOR %s' % filename, file) From f5d9eb15f8b50c64c44a7f859a953b92d7a33e6b Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Fri, 24 Jan 2020 15:06:40 +0200 Subject: [PATCH 75/81] use `__future__` imports at the begining of the file --- scrapy/utils/test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index fd8f41137..65d24314e 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -2,10 +2,10 @@ This module contains some assorted functions used in tests """ -import asyncio -import os from __future__ import absolute_import from posixpath import split +import asyncio +import os from importlib import import_module from twisted.trial.unittest import SkipTest From 40e0a11aa8dd499f725aaa206643aa36411fd514 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Fri, 24 Jan 2020 15:51:48 +0200 Subject: [PATCH 76/81] Fix Flake8 errors --- scrapy/extensions/feedexport.py | 6 ++---- scrapy/pipelines/files.py | 18 +++++++++--------- scrapy/utils/ftp.py | 3 ++- scrapy/utils/test.py | 8 +++++--- tests/test_pipeline_files.py | 5 ++++- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index c1ca0ca67..f1b101780 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -7,18 +7,16 @@ See documentation in docs/topics/feed-exports.rst import os import sys import logging -import posixpath from tempfile import NamedTemporaryFile from datetime import datetime from urllib.parse import urlparse, unquote -from ftplib import FTP from zope.interface import Interface, implementer from twisted.internet import defer, threads from w3lib.url import file_uri_to_path from scrapy import signals -from scrapy.utils.ftp import ftp_makedirs_cwd, ftp_store_file +from scrapy.utils.ftp import ftp_store_file from scrapy.exceptions import NotConfigured from scrapy.utils.misc import create_instance, load_object from scrapy.utils.log import failure_to_exc_info @@ -175,7 +173,7 @@ class FTPFeedStorage(BlockingFeedStorage): def _store_in_thread(self, file): ftp_store_file( path=self.path, file=file, host=self.host, - port=self.port, username=self.username, + port=self.port, username=self.username, password=self.password, use_active_mode=self.use_active_mode ) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 2d286eed8..7e9b12c0e 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -28,7 +28,7 @@ from scrapy.utils.python import to_bytes from scrapy.utils.request import referer_str from scrapy.utils.boto import is_botocore from scrapy.utils.datatypes import CaselessDict -from scrapy.utils.ftp import ftp_makedirs_cwd, ftp_store_file +from scrapy.utils.ftp import ftp_store_file logger = logging.getLogger(__name__) @@ -265,25 +265,25 @@ class FTPFilesStore(object): FTP_USERNAME = None FTP_PASSWORD = None USE_ACTIVE_MODE = None - + def __init__(self, uri): assert uri.startswith('ftp://') - u = urlparse(uri) + u = urlparse(uri) self.port = u.port self.host = u.hostname self.port = int(u.port or 21) self.username = u.username or self.FTP_USERNAME self.password = u.password or self.FTP_PASSWORD self.basedir = u.path.rstrip('/') - - def persist_file(self, path, buf, info, meta=None, headers=None): + + def persist_file(self, path, buf, info, meta=None, headers=None): path = '%s/%s' % (self.basedir, path) return threads.deferToThread( ftp_store_file, path=path, file=buf, host=self.host, port=self.port, username=self.username, password=self.password, use_active_mode=self.USE_ACTIVE_MODE ) - + def stat_file(self, path, info): def _stat_file(path): try: @@ -298,8 +298,8 @@ class FTPFilesStore(object): ftp.retrbinary('RETR %s' % file_path, m.update) return {'last_modified': last_modified, 'checksum': m.hexdigest()} # The file doesn't exist - except Exception as e : - return {} + except Exception: + return {} return threads.deferToThread(_stat_file, path) @@ -381,7 +381,7 @@ class FilesPipeline(MediaPipeline): ftp_store.FTP_USERNAME = settings['FTP_USER'] ftp_store.FTP_PASSWORD = settings['FTP_PASSWORD'] ftp_store.USE_ACTIVE_MODE = settings.getbool('FEED_STORAGE_FTP_ACTIVE') - + store_uri = settings['FILES_STORE'] return cls(store_uri, settings=settings) diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index 1bb754a69..f07bdd748 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -17,7 +17,8 @@ def ftp_makedirs_cwd(ftp, path, first_call=True): if first_call: ftp.cwd(path) -def ftp_store_file( + +def ftp_store_file( *, path, file, host, port, username, password, use_active_mode=False): """Opens a FTP connection with passed credentials,sets current directory diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 65d24314e..61f2d059d 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -66,8 +66,9 @@ def get_gcs_content_and_delete(bucket, path): return content, acl, blob -def get_ftp_content_and_delete(path, host ,port, - username, password, use_active_mode=False): +def get_ftp_content_and_delete( + path, host, port,username, + password, use_active_mode=False): from ftplib import FTP ftp = FTP() ftp.connect(host, port) @@ -75,6 +76,7 @@ def get_ftp_content_and_delete(path, host ,port, if use_active_mode: ftp.set_pasv(False) ftp_data = [] + def buffer_data(data): ftp_data.append(data) ftp.retrbinary('RETR %s' % path, buffer_data) @@ -82,7 +84,7 @@ def get_ftp_content_and_delete(path, host ,port, ftp.cwd(dirname) ftp.delete(filename) return "".join(ftp_data) - + def get_crawler(spidercls=None, settings_dict=None): """Return an unconfigured Crawler object. If settings_dict is given, it diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index fc0453a97..e5bad2ed0 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -367,6 +367,7 @@ class TestGCSFilesStore(unittest.TestCase): self.assertEqual(blob.content_type, 'application/octet-stream') self.assertIn(expected_policy, acl) + class TestFTPFileStore(unittest.TestCase): @defer.inlineCallbacks def test_persist(self): @@ -386,10 +387,12 @@ class TestFTPFileStore(unittest.TestCase): self.assertIn('checksum', stat) self.assertEqual(stat['checksum'], 'd113d66b2ec7258724a268bd88eef6b6') path = '%s/%s' % (store.basedir, path) - content = get_ftp_content_and_delete(path, store.host, store.port, + content = get_ftp_content_and_delete( + path, store.host, store.port, store.username, store.password, store.USE_ACTIVE_MODE) self.assertEqual(data.decode(), content) + class ItemWithFiles(Item): file_urls = Field() files = Field() From 9e6d5573f1180bc70d7eca9f381204c238b3a550 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Fri, 24 Jan 2020 15:58:52 +0200 Subject: [PATCH 77/81] Fix Flake8 errors --- scrapy/pipelines/files.py | 3 +-- scrapy/utils/test.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 7e9b12c0e..9b7445755 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -13,7 +13,6 @@ from collections import defaultdict from email.utils import parsedate_tz, mktime_tz from ftplib import FTP from io import BytesIO -from six.moves.urllib.parse import urlparse from urllib.parse import urlparse from twisted.internet import defer, threads @@ -276,7 +275,7 @@ class FTPFilesStore(object): self.password = u.password or self.FTP_PASSWORD self.basedir = u.path.rstrip('/') - def persist_file(self, path, buf, info, meta=None, headers=None): + def persist_file(self, path, buf, info, meta=None, headers=None): path = '%s/%s' % (self.basedir, path) return threads.deferToThread( ftp_store_file, path=path, file=buf, diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 61f2d059d..faac0b12f 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -67,7 +67,7 @@ def get_gcs_content_and_delete(bucket, path): def get_ftp_content_and_delete( - path, host, port,username, + path, host, port, username, password, use_active_mode=False): from ftplib import FTP ftp = FTP() From f3374a50479fcfd9395919b204bcaff2405f7148 Mon Sep 17 00:00:00 2001 From: Peter Vandenabeele Date: Sat, 25 Jan 2020 16:52:30 +0100 Subject: [PATCH 78/81] Fix variable name `author_page_links` I did not test this code, but the change from `href` to this author_page_links seems to have a typo ? --- docs/intro/tutorial.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index c9d00eb74..ee10048b5 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -652,7 +652,7 @@ this time for scraping author information:: def parse(self, response): author_page_links = response.css('.author + a') - yield from response.follow_all(author_links, self.parse_author) + yield from response.follow_all(author_page_links, self.parse_author) pagination_links = response.css('li.next a') yield from response.follow_all(pagination_links, self.parse) From f72d4e93e6b8a6774c515d55ca73fdbd6a01f13b Mon Sep 17 00:00:00 2001 From: Peter Vandenabeele Date: Sun, 26 Jan 2020 10:48:28 +0100 Subject: [PATCH 79/81] [Docs] 2 typos + 1 clarification in docs Fixing 2 small typos and adding 1 word as clarification in the downloader-middlewares. Also, I was confused with the entries like `ref:Reppy ` and similar entries. Are these supposed to be links to other parts of the doc, or is this the intended way of showing these references ? --- docs/topics/downloader-middleware.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index ae6d41809..8a760e53b 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -199,7 +199,7 @@ CookiesMiddleware This middleware enables working with sites that require cookies, such as those that use sessions. It keeps track of cookies sent by web servers, and - send them back on subsequent requests (from that spider), just like web + sends them back on subsequent requests (from that spider), just like web browsers do. The following settings can be used to configure the cookie middleware: @@ -672,7 +672,7 @@ sometimes a more nuanced policy is desirable. This setting still respects ``Cache-Control: no-store`` directives in responses. If you don't want that, filter ``no-store`` out of the Cache-Control headers in -responses you feedto the cache middleware. +responses you feed to the cache middleware. .. setting:: HTTPCACHE_IGNORE_RESPONSE_CACHE_CONTROLS @@ -686,7 +686,7 @@ Default: ``[]`` List of Cache-Control directives in responses to be ignored. Sites often set "no-store", "no-cache", "must-revalidate", etc., but get -upset at the traffic a spider can generate if it respects those +upset at the traffic a spider can generate if it actually respects those directives. This allows to selectively ignore Cache-Control directives that are known to be unimportant for the sites being crawled. From a3b168948cb533427eceb68cf84bc8542732848b Mon Sep 17 00:00:00 2001 From: Kevin Lloyd Bernal Date: Wed, 29 Jan 2020 04:53:25 +0800 Subject: [PATCH 80/81] Log an error when giving up requests after too many retries (#3566) --- scrapy/downloadermiddlewares/retry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/downloadermiddlewares/retry.py b/scrapy/downloadermiddlewares/retry.py index dbc605a4c..7ab5b6e62 100644 --- a/scrapy/downloadermiddlewares/retry.py +++ b/scrapy/downloadermiddlewares/retry.py @@ -84,6 +84,6 @@ class RetryMiddleware(object): return retryreq else: stats.inc_value('retry/max_reached') - logger.debug("Gave up retrying %(request)s (failed %(retries)d times): %(reason)s", + logger.error("Gave up retrying %(request)s (failed %(retries)d times): %(reason)s", {'request': request, 'retries': retries, 'reason': reason}, extra={'spider': spider}) From 4e56571a196c09f8976b1b31a82a8ce2d0ee0be7 Mon Sep 17 00:00:00 2001 From: Evgeny Dorofeev Date: Wed, 29 Jan 2020 15:48:47 +0300 Subject: [PATCH 81/81] [HttpCompressionMiddleware] fix delimiter for Accept-Encoding header --- scrapy/downloadermiddlewares/httpcompression.py | 2 +- tests/test_downloadermiddleware_httpcompression.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 65b652953..0010b2a8f 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -26,7 +26,7 @@ class HttpCompressionMiddleware(object): def process_request(self, request, spider): request.headers.setdefault('Accept-Encoding', - b",".join(ACCEPTED_ENCODINGS)) + b", ".join(ACCEPTED_ENCODINGS)) def process_response(self, request, response, spider): diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index c6a823b53..64488841a 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -48,7 +48,7 @@ class HttpCompressionTest(TestCase): } response = Response('http://scrapytest.org/', body=body, headers=headers) - response.request = Request('http://scrapytest.org', headers={'Accept-Encoding': 'gzip,deflate'}) + response.request = Request('http://scrapytest.org', headers={'Accept-Encoding': 'gzip, deflate'}) return response def test_process_request(self): @@ -56,7 +56,7 @@ class HttpCompressionTest(TestCase): assert 'Accept-Encoding' not in request.headers self.mw.process_request(request, self.spider) self.assertEqual(request.headers.get('Accept-Encoding'), - b','.join(ACCEPTED_ENCODINGS)) + b', '.join(ACCEPTED_ENCODINGS)) def test_process_response_gzip(self): response = self._getresponse('gzip')