From 8e8994c6b55fa8e975ce30b25ae326f829a58aed Mon Sep 17 00:00:00 2001 From: rhoboro Date: Mon, 2 Apr 2018 15:36:47 +0900 Subject: [PATCH 1/9] add acl support for gcs --- scrapy/pipelines/files.py | 6 +++++- scrapy/pipelines/images.py | 1 + scrapy/settings/default_settings.py | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index ab18a727d..af1d5488a 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -207,6 +207,8 @@ class GCSFilesStore(object): GCS_PROJECT_ID = None CACHE_CONTROL = 'max-age=172800' + POLICY = 'projectPrivate' # Overriden from settings.FILES_STORE_GCS_ACL in + # FilesPipeline.from_settings. def __init__(self, uri): from google.cloud import storage @@ -239,7 +241,8 @@ class GCSFilesStore(object): return threads.deferToThread( blob.upload_from_string, data=buf.getvalue(), - content_type=self._get_content_type(headers) + content_type=self._get_content_type(headers), + predefined_acl=self.POLICY ) @@ -314,6 +317,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'] store_uri = settings['FILES_STORE'] return cls(store_uri, settings=settings) diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index c5fc12afe..5cdddce49 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -93,6 +93,7 @@ class ImagesPipeline(FilesPipeline): gcs_store = cls.STORE_SCHEMES['gs'] gcs_store.GCS_PROJECT_ID = settings['GCS_PROJECT_ID'] + gcs_store.POLICY = settings['IMAGES_STORE_GCS_ACL'] store_uri = settings['IMAGES_STORE'] return cls(store_uri, settings=settings) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index ead511473..7916b9704 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -159,6 +159,7 @@ FEED_EXPORTERS_BASE = { FEED_EXPORT_INDENT = 0 FILES_STORE_S3_ACL = 'private' +FILES_STORE_GCS_ACL = 'projectPrivate' FTP_USER = 'anonymous' FTP_PASSWORD = 'guest' @@ -181,6 +182,7 @@ HTTPPROXY_ENABLED = True HTTPPROXY_AUTH_ENCODING = 'latin-1' IMAGES_STORE_S3_ACL = 'private' +IMAGES_STORE_GCS_ACL = 'projectPrivate' ITEM_PROCESSOR = 'scrapy.pipelines.ItemPipelineManager' From 5254ac393bdf712db698bfa41caf5fb2e682f883 Mon Sep 17 00:00:00 2001 From: rhoboro Date: Tue, 3 Apr 2018 18:00:08 +0900 Subject: [PATCH 2/9] added test for gcs policy --- scrapy/utils/test.py | 3 ++- tests/test_pipeline_files.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 60b931f48..4b935c51b 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -57,8 +57,9 @@ def get_gcs_content_and_delete(bucket, path): bucket = client.get_bucket(bucket) blob = bucket.get_blob(path) content = blob.download_as_string() + acl = list(blob.acl) # loads acl before it will be deleted bucket.delete_blob(path) - return content, blob + return content, acl, blob 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 c761bd606..728a74803 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -388,17 +388,20 @@ class TestGCSFilesStore(unittest.TestCase): meta = {'foo': 'bar'} path = 'full/filename' store = GCSFilesStore(uri) + store.POLICY = 'authenticatedRead' + expected_policy = {'role': 'READER', 'entity': 'allAuthenticatedUsers'} yield store.persist_file(path, buf, info=None, meta=meta, headers=None) s = yield store.stat_file(path, info=None) self.assertIn('last_modified', s) self.assertIn('checksum', s) self.assertEqual(s['checksum'], 'zc2oVgXkbQr2EQdSdw3OPA==') u = urlparse(uri) - content, blob = get_gcs_content_and_delete(u.hostname, u.path[1:]+path) + content, acl, blob = get_gcs_content_and_delete(u.hostname, u.path[1:]+path) self.assertEqual(content, data) self.assertEqual(blob.metadata, {'foo': 'bar'}) self.assertEqual(blob.cache_control, GCSFilesStore.CACHE_CONTROL) self.assertEqual(blob.content_type, 'application/octet-stream') + self.assertIn(expected_policy, acl) class ItemWithFiles(Item): From 74a9c65290888b3ce712c3cfa7132f91ffa3c576 Mon Sep 17 00:00:00 2001 From: rhoboro Date: Tue, 3 Apr 2018 18:20:37 +0900 Subject: [PATCH 3/9] update docs for support gcs acl --- docs/topics/media-pipeline.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 41beebe98..284ec1a25 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -189,6 +189,8 @@ Google Cloud Storage --------------------- .. setting:: GCS_PROJECT_ID +.. setting:: FILES_STORE_GCS_ACL +.. setting:: IMAGES_STORE_GCS_ACL :setting:`FILES_STORE` and :setting:`IMAGES_STORE` can represent a Google Cloud Storage bucket. Scrapy will automatically upload the files to the bucket. (requires `google-cloud-storage`_ ) @@ -204,6 +206,18 @@ For information about authentication, see this `documentation`_. .. _documentation: https://cloud.google.com/docs/authentication/production +You can modify the Access Control List (ACL) policy used for the stored files, +which is defined by the :setting:`FILES_STORE_GCS_ACL` and +:setting:`IMAGES_STORE_GCS_ACL` settings. By default, the ACL is set to +``projectPrivate``. To make the files publicly available use the ``publicRead`` +policy:: + + IMAGES_STORE_GCS_ACL = 'publicRead' + +For more information, see `Predefined ACLs`_ in the Google Cloud Platform Developer Guide. + +.. _Predefined ACLs: https://cloud.google.com/storage/docs/access-control/lists#predefined-acl + Usage example ============= From 464973489e9f457570d7dd1a2e4aab8c4c1778fd Mon Sep 17 00:00:00 2001 From: rhoboro Date: Fri, 13 Apr 2018 12:06:39 +0900 Subject: [PATCH 4/9] Using bucket's default object ACL --- docs/topics/media-pipeline.rst | 3 ++- scrapy/pipelines/files.py | 6 ++++-- scrapy/settings/default_settings.py | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 284ec1a25..0872ac0cd 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -209,7 +209,8 @@ For information about authentication, see this `documentation`_. You can modify the Access Control List (ACL) policy used for the stored files, which is defined by the :setting:`FILES_STORE_GCS_ACL` and :setting:`IMAGES_STORE_GCS_ACL` settings. By default, the ACL is set to -``projectPrivate``. To make the files publicly available use the ``publicRead`` +None which means that Cloud Storage applies the bucket's default object ACL to the object. +To make the files publicly available use the ``publicRead`` policy:: IMAGES_STORE_GCS_ACL = 'publicRead' diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index af1d5488a..8ea70e5d1 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -207,8 +207,10 @@ class GCSFilesStore(object): GCS_PROJECT_ID = None CACHE_CONTROL = 'max-age=172800' - POLICY = 'projectPrivate' # Overriden from settings.FILES_STORE_GCS_ACL in - # FilesPipeline.from_settings. + + # The bucket's default object ACL will be applied to the object. + # Overriden from settings.FILES_STORE_GCS_ACL in FilesPipeline.from_settings. + POLICY = None def __init__(self, uri): from google.cloud import storage diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 7916b9704..d7ca8a835 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -159,7 +159,7 @@ FEED_EXPORTERS_BASE = { FEED_EXPORT_INDENT = 0 FILES_STORE_S3_ACL = 'private' -FILES_STORE_GCS_ACL = 'projectPrivate' +FILES_STORE_GCS_ACL = None FTP_USER = 'anonymous' FTP_PASSWORD = 'guest' @@ -182,7 +182,7 @@ HTTPPROXY_ENABLED = True HTTPPROXY_AUTH_ENCODING = 'latin-1' IMAGES_STORE_S3_ACL = 'private' -IMAGES_STORE_GCS_ACL = 'projectPrivate' +IMAGES_STORE_GCS_ACL = None ITEM_PROCESSOR = 'scrapy.pipelines.ItemPipelineManager' From 560ee623fd84c3db986fccacaef7e9a31a8ea02c Mon Sep 17 00:00:00 2001 From: rhoboro Date: Fri, 13 Apr 2018 19:00:27 +0900 Subject: [PATCH 5/9] set defalut value "" to FILES_STORE_GCS_ACL --- scrapy/pipelines/files.py | 2 +- scrapy/pipelines/images.py | 2 +- scrapy/settings/default_settings.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 8ea70e5d1..510cc23c7 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -319,7 +319,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'] + gcs_store.POLICY = settings['FILES_STORE_GCS_ACL'] or None store_uri = settings['FILES_STORE'] return cls(store_uri, settings=settings) diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 5cdddce49..95323c613 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -93,7 +93,7 @@ class ImagesPipeline(FilesPipeline): gcs_store = cls.STORE_SCHEMES['gs'] gcs_store.GCS_PROJECT_ID = settings['GCS_PROJECT_ID'] - gcs_store.POLICY = settings['IMAGES_STORE_GCS_ACL'] + gcs_store.POLICY = settings['IMAGES_STORE_GCS_ACL'] or None store_uri = settings['IMAGES_STORE'] return cls(store_uri, settings=settings) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index d7ca8a835..36e17ef6b 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -159,7 +159,7 @@ FEED_EXPORTERS_BASE = { FEED_EXPORT_INDENT = 0 FILES_STORE_S3_ACL = 'private' -FILES_STORE_GCS_ACL = None +FILES_STORE_GCS_ACL = '' FTP_USER = 'anonymous' FTP_PASSWORD = 'guest' @@ -182,7 +182,7 @@ HTTPPROXY_ENABLED = True HTTPPROXY_AUTH_ENCODING = 'latin-1' IMAGES_STORE_S3_ACL = 'private' -IMAGES_STORE_GCS_ACL = None +IMAGES_STORE_GCS_ACL = '' ITEM_PROCESSOR = 'scrapy.pipelines.ItemPipelineManager' From 6ef6585b5a187d4a8dcec99ba7cae9b6cee91b30 Mon Sep 17 00:00:00 2001 From: rhoboro Date: Fri, 13 Apr 2018 19:06:29 +0900 Subject: [PATCH 6/9] update docs --- 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 0872ac0cd..a1f518cbd 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -209,7 +209,7 @@ For information about authentication, see this `documentation`_. You can modify the Access Control List (ACL) policy used for the stored files, which is defined by the :setting:`FILES_STORE_GCS_ACL` and :setting:`IMAGES_STORE_GCS_ACL` settings. By default, the ACL is set to -None which means that Cloud Storage applies the bucket's default object ACL to the object. +``''`` (empty string) which means that Cloud Storage applies the bucket's default object ACL to the object. To make the files publicly available use the ``publicRead`` policy:: From 57b0e6b6955705efcbe2d3da5501b27716ef1baf Mon Sep 17 00:00:00 2001 From: Lucy Wang Date: Thu, 19 Apr 2018 13:35:46 +0800 Subject: [PATCH 7/9] improve document about functions as processors --- docs/topics/loaders.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/topics/loaders.rst b/docs/topics/loaders.rst index ad86dba63..cdb066e57 100644 --- a/docs/topics/loaders.rst +++ b/docs/topics/loaders.rst @@ -136,6 +136,20 @@ accept one (and only one) positional argument, which will be an iterator. containing the collected values (for that field). The result of the output processors is the value that will be finally assigned to the item. +If you want to use a plain function as a processor, make sure it receives +``self`` as the first argument:: + + def lowercase_processor(self, values): + for v in values: + yield v.lower() + + class MyItemLoader(ItemLoader): + name_in = lowercase_processor + +This is because whenever a function is assigned as a class variable, it becomes +a method and would be passed the instance as the the first argument when being +called. See `this answer on stackoverflow`_ for more details. + The other thing you need to keep in mind is that the values returned by input processors are collected internally (in lists) and then passed to output processors to populate the fields. @@ -143,6 +157,7 @@ processors to populate the fields. Last, but not least, Scrapy comes with some :ref:`commonly used processors ` built-in for convenience. +.. _this answer on stackoverflow: https://stackoverflow.com/a/35322635 Declaring Item Loaders ====================== From 0d015e5c0f0dcb8936044aec163dd12e33482730 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Wed, 16 May 2018 09:36:07 +0000 Subject: [PATCH 8/9] blacklist twisted version with regression --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 2a94d742d..7d857a8c9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -Twisted>=13.1.0 +Twisted>=13.1.0,!=18.4.0 lxml pyOpenSSL cssselect>=0.9 From c5ddfddb7e5b42e45b6c8f86a381ee52fbb6f797 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Thu, 17 May 2018 08:53:42 +0000 Subject: [PATCH 9/9] blacklist twisted version with regression in constraints file --- requirements.txt | 2 +- tests/constraints.txt | 1 + tox.ini | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 tests/constraints.txt diff --git a/requirements.txt b/requirements.txt index 7d857a8c9..2a94d742d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -Twisted>=13.1.0,!=18.4.0 +Twisted>=13.1.0 lxml pyOpenSSL cssselect>=0.9 diff --git a/tests/constraints.txt b/tests/constraints.txt new file mode 100644 index 000000000..3bc30de15 --- /dev/null +++ b/tests/constraints.txt @@ -0,0 +1 @@ +Twisted!=18.4.0 diff --git a/tox.ini b/tox.ini index 60ff8c15e..c2fa9af28 100644 --- a/tox.ini +++ b/tox.ini @@ -8,6 +8,7 @@ envlist = py27 [testenv] deps = + -ctests/constraints.txt -rrequirements.txt # Extras botocore