From c025003da2c60a0a786c2ee158fe620402914273 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Sun, 18 Aug 2019 04:44:09 +0200 Subject: [PATCH 01/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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 8ea8f14827470f37c0e53d302aa65bcfa9604f3c Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Mon, 20 Jan 2020 18:19:36 +0200 Subject: [PATCH 14/19] 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 15/19] 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 c544c0d2b8356125d1a5465b44617aaaaeab0ea1 Mon Sep 17 00:00:00 2001 From: OmarFarrag Date: Fri, 24 Jan 2020 14:36:16 +0200 Subject: [PATCH 16/19] 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 17/19] 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 18/19] 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 19/19] 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()