From 2a540206a74af8d38a01aaa5a37adc1008cad6ca Mon Sep 17 00:00:00 2001 From: nramirezuy Date: Tue, 19 Aug 2014 13:57:00 -0300 Subject: [PATCH 01/34] fix xmliter namespace on selected node --- scrapy/utils/iterators.py | 22 +++++++++++++++------- tests/test_utils_iterators.py | 6 +++++- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index 150b077ae..11b873f2e 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -20,19 +20,27 @@ def xmliter(obj, nodename): - a unicode string - a string encoded as utf-8 """ - HEADER_START_RE = re.compile(r'^(.*?)<\s*%s(?:\s|>)' % nodename, re.S) + DOCUMENT_HEADER_RE = re.compile(r'<\?xml[^>]+>\s*', re.S) HEADER_END_RE = re.compile(r'<\s*/%s\s*>' % nodename, re.S) + END_TAG_RE = re.compile(r'<\s*/([^\s>]+)\s*>', re.S) + NAMESPACE_RE = re.compile(r'((xmlns[:A-Za-z]*)=[^>\s]+)', re.S) text = _body_or_str(obj) - header_start = re.search(HEADER_START_RE, text) - header_start = header_start.group(1).strip() if header_start else '' - header_end = re_rsearch(HEADER_END_RE, text) - header_end = text[header_end[1]:].strip() if header_end else '' + document_header = re.search(DOCUMENT_HEADER_RE, text) + document_header = document_header.group().strip() if document_header else '' + header_end_idx = re_rsearch(HEADER_END_RE, text) + header_end = text[header_end_idx[1]:].strip() if header_end_idx else '' + namespaces = {} + if header_end: + for tagname in reversed(re.findall(END_TAG_RE, header_end)): + tag = re.search(r'<\s*%s.*?xmlns[:=][^>]*>' % tagname, text[:header_end_idx[1]], re.S) + if tag: + namespaces.update(reversed(x) for x in re.findall(NAMESPACE_RE, tag.group())) r = re.compile(r"<%s[\s>].*?" % (nodename, nodename), re.DOTALL) for match in r.finditer(text): - nodetext = header_start + match.group() + header_end - yield Selector(text=nodetext, type='xml').xpath('//' + nodename)[0] + nodetext = document_header + match.group().replace(nodename, '%s %s' % (nodename, ' '.join(namespaces.values())), 1) + header_end + yield Selector(text=nodetext, type='xml') def csviter(obj, delimiter=None, headers=None, encoding=None): diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index fe53f831f..8b5941605 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -61,7 +61,6 @@ class XmliterTestCase(unittest.TestCase): """ response = XmlResponse(url='http://mydummycompany.com', body=body) my_iter = self.xmliter(response, 'item') - node = next(my_iter) node.register_namespace('g', 'http://base.google.com/ns/1.0') self.assertEqual(node.xpath('title/text()').extract(), ['Item 1']) @@ -74,6 +73,11 @@ class XmliterTestCase(unittest.TestCase): self.assertEqual(node.xpath('id/text()').extract(), []) self.assertEqual(node.xpath('price/text()').extract(), []) + my_iter = self.xmliter(response, 'g:image_link') + node = next(my_iter) + node.register_namespace('g', 'http://base.google.com/ns/1.0') + self.assertEqual(node.xpath('text()').extract(), ['http://www.mydummycompany.com/images/item1.jpg']) + def test_xmliter_exception(self): body = u"""onetwo""" From e90be0d8a5e3c20ab6aced22ccac59a876db8c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 21 Aug 2020 14:09:52 +0200 Subject: [PATCH 02/34] Mark the new test as xfail for xmliter_lxml --- tests/test_utils_iterators.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index bbdc88dd1..ae64e36cf 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -1,5 +1,6 @@ import os +from pytest import mark from twisted.trial import unittest from scrapy.utils.iterators import csviter, xmliter, _body_or_str, xmliter_lxml @@ -149,6 +150,26 @@ class XmliterTestCase(unittest.TestCase): self.assertEqual(node.xpath('id/text()').getall(), []) self.assertEqual(node.xpath('price/text()').getall(), []) + def test_xmliter_namespaced_nodename(self): + body = b""" + + + + My Dummy Company + http://www.mydummycompany.com + This is a dummy company. We do nothing. + + Item 1 + This is item 1 + http://www.mydummycompany.com/items/1 + http://www.mydummycompany.com/images/item1.jpg + ITEM_1 + 400 + + + + """ + response = XmlResponse(url='http://mydummycompany.com', body=body) my_iter = self.xmliter(response, 'g:image_link') node = next(my_iter) node.register_namespace('g', 'http://base.google.com/ns/1.0') @@ -187,6 +208,10 @@ class XmliterTestCase(unittest.TestCase): class LxmlXmliterTestCase(XmliterTestCase): xmliter = staticmethod(xmliter_lxml) + @mark.xfail(reason='known bug of the current implementation') + def test_xmliter_namespaced_nodename(self): + super().test_xmliter_namespaced_nodename() + def test_xmliter_iterate_namespace(self): body = b""" From afd3a4d116809ecf4c334657c5150c157aa9465b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 21 Aug 2020 17:04:02 +0200 Subject: [PATCH 03/34] Fix style issue --- scrapy/utils/iterators.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index c356ad7f8..6f6b5e337 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -41,7 +41,15 @@ def xmliter(obj, nodename): r = re.compile(r'<%(np)s[\s>].*?' % {'np': nodename_patt}, re.DOTALL) for match in r.finditer(text): - nodetext = document_header + match.group().replace(nodename, '%s %s' % (nodename, ' '.join(namespaces.values())), 1) + header_end + nodetext = ( + document_header + + match.group().replace( + nodename, + '%s %s' % (nodename, ' '.join(namespaces.values())), + 1 + ) + + header_end + ) yield Selector(text=nodetext, type='xml') From 2f28cee3ce482a9380c816b689fc6a5dabc60295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 25 Aug 2020 17:49:17 +0200 Subject: [PATCH 04/34] Add a test to cover searching for a missing node name --- tests/test_utils_iterators.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index ae64e36cf..2adccebb8 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -175,6 +175,30 @@ class XmliterTestCase(unittest.TestCase): node.register_namespace('g', 'http://base.google.com/ns/1.0') self.assertEqual(node.xpath('text()').extract(), ['http://www.mydummycompany.com/images/item1.jpg']) + def test_xmliter_namespaced_nodename_missing(self): + body = b""" + + + + My Dummy Company + http://www.mydummycompany.com + This is a dummy company. We do nothing. + + Item 1 + This is item 1 + http://www.mydummycompany.com/items/1 + http://www.mydummycompany.com/images/item1.jpg + ITEM_1 + 400 + + + + """ + response = XmlResponse(url='http://mydummycompany.com', body=body) + my_iter = self.xmliter(response, 'g:link_image') + with self.assertRaises(StopIteration): + next(my_iter) + def test_xmliter_exception(self): body = ( '' From dd378b4bb1499b9e8da7b193f19ae6a8d81b5a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 27 Aug 2020 11:07:58 +0200 Subject: [PATCH 05/34] Generate localhost keys for tests on the fly --- .gitignore | 2 ++ conftest.py | 6 ++++ tests/keys/__init__.py | 63 ++++++++++++++++++++++++++++++++++++++++ tests/keys/localhost.crt | 20 ------------- tests/keys/localhost.key | 28 ------------------ 5 files changed, 71 insertions(+), 48 deletions(-) create mode 100644 tests/keys/__init__.py delete mode 100644 tests/keys/localhost.crt delete mode 100644 tests/keys/localhost.key diff --git a/.gitignore b/.gitignore index 83a2569dd..795e2605e 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,8 @@ htmlcov/ .coverage.* .cache/ .mypy_cache/ +/tests/keys/localhost.crt +/tests/keys/localhost.key # Windows Thumbs.db diff --git a/conftest.py b/conftest.py index b39d644a5..95d4eaef6 100644 --- a/conftest.py +++ b/conftest.py @@ -2,6 +2,8 @@ from pathlib import Path import pytest +from tests.keys import generate_keys + def _py_files(folder): return (str(p) for p in Path(folder).rglob('*.py')) @@ -53,3 +55,7 @@ def reactor_pytest(request): def only_asyncio(request, reactor_pytest): if request.node.get_closest_marker('only_asyncio') and reactor_pytest != 'asyncio': pytest.skip('This test is only run with --reactor=asyncio') + + +# Generate localhost certificate files, needed by some tests +generate_keys() diff --git a/tests/keys/__init__.py b/tests/keys/__init__.py new file mode 100644 index 000000000..da202be4d --- /dev/null +++ b/tests/keys/__init__.py @@ -0,0 +1,63 @@ +import os +from datetime import datetime, timedelta + +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.hazmat.primitives.hashes import SHA256 +from cryptography.hazmat.primitives.serialization import ( + Encoding, + NoEncryption, + PrivateFormat, +) +from cryptography.x509 import ( + CertificateBuilder, + DNSName, + Name, + NameAttribute, + random_serial_number, + SubjectAlternativeName, +) +from cryptography.x509.oid import NameOID + + +# https://cryptography.io/en/latest/x509/tutorial/#creating-a-self-signed-certificate +def generate_keys(): + folder = os.path.dirname(__file__) + + key = rsa.generate_private_key( + public_exponent=65537, + key_size=2048, + backend=default_backend(), + ) + with open(os.path.join(folder, 'localhost.key'), "wb") as f: + f.write( + key.private_bytes( + encoding=Encoding.PEM, + format=PrivateFormat.TraditionalOpenSSL, + encryption_algorithm=NoEncryption(), + ) + ) + + subject = issuer = Name( + [ + NameAttribute(NameOID.COUNTRY_NAME, u"IE"), + NameAttribute(NameOID.ORGANIZATION_NAME, u"Scrapy"), + NameAttribute(NameOID.COMMON_NAME, u"localhost"), + ] + ) + cert = ( + CertificateBuilder() + .subject_name(subject) + .issuer_name(issuer) + .public_key(key.public_key()) + .serial_number(random_serial_number()) + .not_valid_before(datetime.utcnow()) + .not_valid_after(datetime.utcnow() + timedelta(days=10)) + .add_extension( + SubjectAlternativeName([DNSName(u"localhost")]), + critical=False, + ) + .sign(key, SHA256(), default_backend()) + ) + with open(os.path.join(folder, 'localhost.crt'), "wb") as f: + f.write(cert.public_bytes(Encoding.PEM)) diff --git a/tests/keys/localhost.crt b/tests/keys/localhost.crt deleted file mode 100644 index 0cf5256d8..000000000 --- a/tests/keys/localhost.crt +++ /dev/null @@ -1,20 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIDRTCCAi2gAwIBAgIUGoISfeW3LwSWHC52ORXdZY9pNLswDQYJKoZIhvcNAQEL -BQAwMjELMAkGA1UEBhMCSUUxDzANBgNVBAoMBlNjcmFweTESMBAGA1UEAwwJbG9j -YWxob3N0MB4XDTIwMDYyODEyNTQxNVoXDTIxMDYyODEyNTQxNVowMjELMAkGA1UE -BhMCSUUxDzANBgNVBAoMBlNjcmFweTESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjAN -BgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvCLxfTEQuIdf8JhiHrbVkGHYrNSK -2XD2TCPaSIpJ2KKlFUrIz3A9tWlOfLnWabS5od89yOebhYj4DN/Qm2TViGg1mtWe -pD1K2YWd1Af+hhAw5D+TpW2RH9TVhX7Ey5osWcl+0uy+RlKZE8qum72xi1vxWOmH -wYw06iN8klQ3JfP2/eLRXBQjsh7WW0dbJ7yLvG6UFz1RbhFTtlxeIMenzNsHaMg7 -56Ru57/MMbaBwdBttXVzJDQ7imo8njuxDMszliC/QgIdBUBFzA2LB5qpr+v+laDN -cN9t9Q9stsu446dFnRoofxJjMFW7lLu6h/lwP5r0kfeUkMDhXJ4mb6KwfwIDAQAB -o1MwUTAdBgNVHQ4EFgQUVEdXn8ha2FA73zcy1Ia0FQMzMEYwHwYDVR0jBBgwFoAU -VEdXn8ha2FA73zcy1Ia0FQMzMEYwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0B -AQsFAAOCAQEAZpGBPsexMD+IwcMNIgc7FiaJsb8E30C9vWxgdnkpapi9zLJ4yiHQ -VxkV9RTezUEADkaDj+2qFveamWTzJLnphgaaUpVeMcYACPhRVOYXidNrZyTmHIsX -FwaTzAggW6CP7JxAcpxH0f9+NWFCZI36FihRdwuWyvrUl7rsXaexu0SOI/Ck0oWf -2IW+jo67TSmcbte+J8wq77DX32mVLb/2nqpItH4T2Di+XjVBARACVOSdgdlo7lZE -W8mSEXqP2BVx8JGG8X1znNLHcmjVj4EtkpH0wkYzpC4cvGkTsUcU7CU7ZyVUp+Bb -dPMVxyRKWfAjRJc8o5Ot1mgHrx5coOtzAA== ------END CERTIFICATE----- diff --git a/tests/keys/localhost.key b/tests/keys/localhost.key deleted file mode 100644 index 8fc373bdd..000000000 --- a/tests/keys/localhost.key +++ /dev/null @@ -1,28 +0,0 @@ ------BEGIN PRIVATE KEY----- -MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQC8IvF9MRC4h1/w -mGIettWQYdis1IrZcPZMI9pIiknYoqUVSsjPcD21aU58udZptLmh3z3I55uFiPgM -39CbZNWIaDWa1Z6kPUrZhZ3UB/6GEDDkP5OlbZEf1NWFfsTLmixZyX7S7L5GUpkT -yq6bvbGLW/FY6YfBjDTqI3ySVDcl8/b94tFcFCOyHtZbR1snvIu8bpQXPVFuEVO2 -XF4gx6fM2wdoyDvnpG7nv8wxtoHB0G21dXMkNDuKajyeO7EMyzOWIL9CAh0FQEXM -DYsHmqmv6/6VoM1w3231D2y2y7jjp0WdGih/EmMwVbuUu7qH+XA/mvSR95SQwOFc -niZvorB/AgMBAAECggEAHVpSVRb/pdqxNEeCH4qlHWa2uJhcpXpDYzPAzcqNpPgT -S5QkaoD3j8NDVKBl/I4O3FuJNzwzfo0VLmUJFgWQbzzbCDJGExfhArkfG8K3ilEi -X6ovrgK/PrklKzPRHncKbmPKnrwDH9OpQHZB8diRx81rhVTCModehh1NRUNQa2I1 -QzFC7uyXx3duoIsI5QXVeEGuwHZfqIY/z+9SscdVFL6elXTPFUzBzcmAqQgdgWKN -HXgX22LE0rAu8NnRvOZZWt4/nOjvlCFCPTB11NgthmKlVnsx4H7gpQ2OPh4bZ+0W -birVEtZ3E1jxoGvw1FzxyqqpGkcanRMa8QWzK4JwuQKBgQDrgclpkqZrgHB/TC1p -hLvsdflGI2SGs+c/mYR3GEjf0kJtI88WL5fj1QezdkDyOpwxFvnLslswfzdtzvis -vksGysV35vhMPQUcmWhvzA7Pdxdv4BZr+ckER0SAYBBxg9KYZyxewGb5XzB8Cz2o -8V+YpwrMAOYGuXHTfafv4CKlTQKBgQDMgetvV9/E3HNtKsATiPIwT3e1MzyPXigq -12NkHSZa6s4yqm/h/fSUn54sJbhx+OtRRhktOo0aB34tcogtrJyClvCPdRAP/4Qi -M43FjKo2cWiubWvtWlOZU04bpClG324q420rK7dCA2stID/Fa0sMQgAAyPH8TGMo -gbvyrk4W+wKBgQDMIOnYZTF0epaH8BponJFaqwMOhTzr+OGW4dTMebMotZG4EdK8 -kzIfW5XaOsSecKjTb+vCYGzkA1CjEEPBTwuu7nDstblAM5/Lozi/tmqb7sjUwrIM -kyxmVfONJjb6fV07lioCUtiui5B15DRkzBqlMRyNqLW43GJKA19d7rN4/QKBgCzy -kRBTu/bEjQn9T2H7w18i2CiXLkREaYeg91NVpMxutwsjspt0+YCA5H7He5ZxIycl -xPrP15tU8kKC3bNMMMny6sRc8j7R5fSuaAZ3OCHnIx7TJdlw9NbKHGyu0/Ojv87l -VWUbopd7sN6mK930CvaSuvVxNN5C27hXazuXW8ppAoGBANcWsenNKpCJgF0cNPHX -abPaWfcs5FKMNz8gEdGk3B1z/KBpYz59smPwurYVCXaWE6iv99sDOP7CVneF02sV -SqyNzVhcVSG788uB3CwnpEvm7ydoH89L5dvYekAHP8RJulhWCK45lXkHLiYGKvhv -PWuPk5VX+qF78JhUhPO3nfnu ------END PRIVATE KEY----- From a8e895e684184e967a751ea28a7102f21dd74834 Mon Sep 17 00:00:00 2001 From: maranqz Date: Sun, 30 Aug 2020 10:57:22 +0300 Subject: [PATCH 06/34] kwargs for Item exporters classes test docs --- docs/topics/feed-exports.rst | 5 +++++ scrapy/extensions/feedexport.py | 1 + scrapy/utils/conf.py | 1 + tests/test_feedexport.py | 37 +++++++++++++++++++++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 9fb2189e8..e69a64195 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -303,6 +303,9 @@ For instance:: 'store_empty': False, 'fields': None, 'indent': 4, + 'item_export_kwargs': { + 'export_empty_fields': True, + }, }, '/home/user/documents/items.xml': { 'format': 'xml', @@ -332,6 +335,8 @@ as a fallback value if that key is not provided for a specific feed definition: - ``indent``: falls back to :setting:`FEED_EXPORT_INDENT`. +- ``item_export_kwargs``: dict with kwargs for :ref:`Item exporters ` classes. + - ``overwrite``: whether to overwrite the file if it already exists (``True``) or append to its content (``False``). diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 980825499..f32d48fa5 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -360,6 +360,7 @@ class FeedExporter: fields_to_export=feed_options['fields'], encoding=feed_options['encoding'], indent=feed_options['indent'], + **feed_options['item_export_kwargs'], ) slot = _FeedSlot( file=file, diff --git a/scrapy/utils/conf.py b/scrapy/utils/conf.py index 4e7a9967e..b904c4a03 100644 --- a/scrapy/utils/conf.py +++ b/scrapy/utils/conf.py @@ -121,6 +121,7 @@ def feed_complete_default_values_from_settings(feed, settings): out.setdefault("fields", settings.getlist("FEED_EXPORT_FIELDS") or None) out.setdefault("store_empty", settings.getbool("FEED_STORE_EMPTY")) out.setdefault("uri_params", settings["FEED_URI_PARAMS"]) + out.setdefault("item_export_kwargs", dict()) if settings["FEED_EXPORT_INDENT"] is None: out.setdefault("indent", None) else: diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 840e0f87b..e88e4f5ce 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -1252,6 +1252,43 @@ class FeedExportTest(FeedExportTestBase): for fmt in ['json', 'xml', 'csv']: self.assertIn(f'Error storing {fmt} feed (2 items)', str(log)) + @defer.inlineCallbacks + def test_extend_kwargs(self): + items = [{'foo': 'FOO', 'bar': 'BAR'}] + + expected_with_title_csv = 'foo,bar\r\nFOO,BAR\r\n'.encode('utf-8') + expected_without_title_csv = 'FOO,BAR\r\n'.encode('utf-8') + test_cases = [ + # with title + { + 'options': { + 'format': 'csv', + 'item_export_kwargs': dict(include_headers_line=True), + }, + 'expected': expected_with_title_csv, + }, + # without title + { + 'options': { + 'format': 'csv', + 'item_export_kwargs': dict(include_headers_line=False), + }, + 'expected': expected_without_title_csv, + }, + ] + + for row in test_cases: + feed_options = row['options'] + settings = { + 'FEEDS': { + self._random_temp_filename(): feed_options, + }, + 'FEED_EXPORT_INDENT': None, + } + + data = yield self.exported_data(items, settings) + self.assertEqual(row['expected'], data[feed_options['format']]) + class BatchDeliveriesTest(FeedExportTestBase): __test__ = True From fc3c66ce950e945e7ac6e19fea31e8454147ac29 Mon Sep 17 00:00:00 2001 From: maranqz Date: Sun, 30 Aug 2020 11:44:48 +0300 Subject: [PATCH 07/34] fix tests: * FeedExportConfigTestCase.test_feed_complete_default_values_from_settings_empty * FeedExportConfigTestCase.test_feed_complete_default_values_from_settings_non_empty --- tests/test_utils_conf.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_utils_conf.py b/tests/test_utils_conf.py index 061bc8c7c..dc2560add 100644 --- a/tests/test_utils_conf.py +++ b/tests/test_utils_conf.py @@ -176,6 +176,7 @@ class FeedExportConfigTestCase(unittest.TestCase): "store_empty": True, "uri_params": (1, 2, 3, 4), "batch_item_count": 2, + "item_export_kwargs": dict(), }) def test_feed_complete_default_values_from_settings_non_empty(self): @@ -198,6 +199,7 @@ class FeedExportConfigTestCase(unittest.TestCase): "store_empty": True, "uri_params": None, "batch_item_count": 2, + "item_export_kwargs": dict(), }) From 71d2c2f1a319b570eb2026707d49c4f62d59296e Mon Sep 17 00:00:00 2001 From: maranqz Date: Sun, 30 Aug 2020 12:43:44 +0300 Subject: [PATCH 08/34] improve view of dict --- tests/test_feedexport.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index e88e4f5ce..5738e36f1 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -1263,7 +1263,7 @@ class FeedExportTest(FeedExportTestBase): { 'options': { 'format': 'csv', - 'item_export_kwargs': dict(include_headers_line=True), + 'item_export_kwargs': {'include_headers_line': True}, }, 'expected': expected_with_title_csv, }, @@ -1271,7 +1271,7 @@ class FeedExportTest(FeedExportTestBase): { 'options': { 'format': 'csv', - 'item_export_kwargs': dict(include_headers_line=False), + 'item_export_kwargs': {'include_headers_line': False}, }, 'expected': expected_without_title_csv, }, From d10464ca96a24d37c854992e2485d622e8eec2b6 Mon Sep 17 00:00:00 2001 From: Ilia Sergunin Date: Tue, 1 Sep 2020 10:13:40 +0300 Subject: [PATCH 09/34] Update docs/topics/feed-exports.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adrián Chaves --- docs/topics/feed-exports.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index e69a64195..1744cfd74 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -335,7 +335,7 @@ as a fallback value if that key is not provided for a specific feed definition: - ``indent``: falls back to :setting:`FEED_EXPORT_INDENT`. -- ``item_export_kwargs``: dict with kwargs for :ref:`Item exporters ` classes. +- ``item_export_kwargs``: :class:`dict` with keyword arguments for the corresponding :ref:`item exporter class `. - ``overwrite``: whether to overwrite the file if it already exists (``True``) or append to its content (``False``). From 307e35c6641c0d9cf4b251996ebefd1347c2c1fd Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <1731933+elacuesta@users.noreply.github.com> Date: Tue, 1 Sep 2020 06:04:00 -0300 Subject: [PATCH 10/34] Improve check for invalid cookie in CookiesMiddleware (#4772) --- scrapy/downloadermiddlewares/cookies.py | 2 +- tests/test_downloadermiddleware_cookies.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/scrapy/downloadermiddlewares/cookies.py b/scrapy/downloadermiddlewares/cookies.py index e2b7dd901..87f8152a4 100644 --- a/scrapy/downloadermiddlewares/cookies.py +++ b/scrapy/downloadermiddlewares/cookies.py @@ -74,7 +74,7 @@ class CookiesMiddleware: """ decoded = {} for key in ("name", "value", "path", "domain"): - if not cookie.get(key): + if cookie.get(key) is None: if key in ("name", "value"): msg = "Invalid cookie found in request {}: {} ('{}' is missing)" logger.warning(msg.format(request, cookie, key)) diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 010577415..a3de307ee 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -322,6 +322,9 @@ class CookiesMiddlewareTest(TestCase): cookies2 = [{'name': 'foo'}, {'name': 'key', 'value': 'value2'}] req2 = Request('http://example.org/2', cookies=cookies2) assert self.mw.process_request(req2, self.spider) is None + cookies3 = [{'name': 'foo', 'value': None}, {'name': 'key', 'value': ''}] + req3 = Request('http://example.org/3', cookies=cookies3) + assert self.mw.process_request(req3, self.spider) is None lc.check( ("scrapy.downloadermiddlewares.cookies", "WARNING", @@ -331,6 +334,11 @@ class CookiesMiddlewareTest(TestCase): "WARNING", "Invalid cookie found in request :" " {'name': 'foo'} ('value' is missing)"), + ("scrapy.downloadermiddlewares.cookies", + "WARNING", + "Invalid cookie found in request :" + " {'name': 'foo', 'value': None} ('value' is missing)"), ) self.assertCookieValEqual(req1.headers['Cookie'], 'key=value1') self.assertCookieValEqual(req2.headers['Cookie'], 'key=value2') + self.assertCookieValEqual(req3.headers['Cookie'], 'key=') From 959222df7e80334810636166a8adb60ef35728de Mon Sep 17 00:00:00 2001 From: drs-11 Date: Sat, 5 Sep 2020 21:32:05 +0530 Subject: [PATCH 11/34] check for unparseable no_proxy values --- scrapy/downloadermiddlewares/httpproxy.py | 7 ++++++- tests/test_downloadermiddleware_httpproxy.py | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/scrapy/downloadermiddlewares/httpproxy.py b/scrapy/downloadermiddlewares/httpproxy.py index 04da11311..d2665b655 100644 --- a/scrapy/downloadermiddlewares/httpproxy.py +++ b/scrapy/downloadermiddlewares/httpproxy.py @@ -13,7 +13,12 @@ class HttpProxyMiddleware: self.auth_encoding = auth_encoding self.proxies = {} for type_, url in getproxies().items(): - self.proxies[type_] = self._get_proxy(url, type_) + try: + self.proxies[type_] = self._get_proxy(url, type_) + # some values such as '/var/run/docker.sock' can't be parsed + # by _parse_proxy and as such should be skipped + except ValueError: + continue @classmethod def from_crawler(cls, crawler): diff --git a/tests/test_downloadermiddleware_httpproxy.py b/tests/test_downloadermiddleware_httpproxy.py index 351631eb8..81d6cc335 100644 --- a/tests/test_downloadermiddleware_httpproxy.py +++ b/tests/test_downloadermiddleware_httpproxy.py @@ -123,7 +123,11 @@ class TestHttpProxyMiddleware(TestCase): def test_no_proxy(self): os.environ['http_proxy'] = 'https://proxy.for.http:3128' + os.environ['no_proxy'] = '/var/run/docker.sock' mw = HttpProxyMiddleware() + # '/var/run/docker.sock' may be used by the user for + # no_proxy value but is not parseable and should be skipped + assert 'no' not in mw.proxies os.environ['no_proxy'] = '*' req = Request('http://noproxy.com') From 7a83474cc534642e8daf581d4b427408343377f2 Mon Sep 17 00:00:00 2001 From: KAILASA's Sri Nithya Priyeshananda <68758690+sripriyesha@users.noreply.github.com> Date: Tue, 8 Sep 2020 17:16:31 +0200 Subject: [PATCH 12/34] add mention of FTP server storage in media storage intro At the beginning of this doc, in "Specifying where to store the media" feature details, FTP server storage mention was missing --- 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 1f995ce14..c2255ea79 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -15,7 +15,7 @@ typically you'll either use the Files Pipeline or the Images Pipeline. Both pipelines implement these features: * Avoid re-downloading media that was downloaded recently -* Specifying where to store the media (filesystem directory, Amazon S3 bucket, +* Specifying where to store the media (filesystem directory, FTP server, Amazon S3 bucket, Google Cloud Storage bucket) The Images Pipeline has a few extra functions for processing images: From 82ba7c8b529e004a62db4681f16a482ae4e769f1 Mon Sep 17 00:00:00 2001 From: drs-11 Date: Thu, 10 Sep 2020 20:56:39 +0530 Subject: [PATCH 13/34] created separate test for invalid no-proxy values --- tests/test_downloadermiddleware_httpproxy.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/test_downloadermiddleware_httpproxy.py b/tests/test_downloadermiddleware_httpproxy.py index 81d6cc335..7c97bf32a 100644 --- a/tests/test_downloadermiddleware_httpproxy.py +++ b/tests/test_downloadermiddleware_httpproxy.py @@ -123,11 +123,7 @@ class TestHttpProxyMiddleware(TestCase): def test_no_proxy(self): os.environ['http_proxy'] = 'https://proxy.for.http:3128' - os.environ['no_proxy'] = '/var/run/docker.sock' mw = HttpProxyMiddleware() - # '/var/run/docker.sock' may be used by the user for - # no_proxy value but is not parseable and should be skipped - assert 'no' not in mw.proxies os.environ['no_proxy'] = '*' req = Request('http://noproxy.com') @@ -149,3 +145,10 @@ class TestHttpProxyMiddleware(TestCase): req = Request('http://noproxy.com', meta={'proxy': 'http://proxy.com'}) assert mw.process_request(req, spider) is None self.assertEqual(req.meta, {'proxy': 'http://proxy.com'}) + + def test_no_proxy_invalid_values(self): + os.environ['no_proxy'] = '/var/run/docker.sock' + mw = HttpProxyMiddleware() + # '/var/run/docker.sock' may be used by the user for + # no_proxy value but is not parseable and should be skipped + assert 'no' not in mw.proxies From 5e997587d9b13344a0afa9bb4cf781829a66ce23 Mon Sep 17 00:00:00 2001 From: Andrey Rahmatullin Date: Sun, 20 Sep 2020 18:06:46 +0500 Subject: [PATCH 14/34] Remove dead boto2 code, deprecate is_botocore() (#4776) --- scrapy/core/downloader/handlers/s3.py | 55 ++++----------- scrapy/extensions/feedexport.py | 35 ++++------ scrapy/pipelines/files.py | 97 +++++++++------------------ scrapy/utils/boto.py | 23 ++++++- scrapy/utils/test.py | 29 +++----- tests/test_feedexport.py | 69 ++----------------- tests/test_pipeline_files.py | 15 ++--- 7 files changed, 96 insertions(+), 227 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 0ef977893..1966570d4 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -2,41 +2,20 @@ from urllib.parse import unquote from scrapy.core.downloader.handlers.http import HTTPDownloadHandler from scrapy.exceptions import NotConfigured -from scrapy.utils.boto import is_botocore +from scrapy.utils.boto import is_botocore_available from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.misc import create_instance -def _get_boto_connection(): - from boto.s3.connection import S3Connection - - class _v19_S3Connection(S3Connection): - """A dummy S3Connection wrapper that doesn't do any synchronous download""" - def _mexe(self, method, bucket, key, headers, *args, **kwargs): - return headers - - class _v20_S3Connection(S3Connection): - """A dummy S3Connection wrapper that doesn't do any synchronous download""" - def _mexe(self, http_request, *args, **kwargs): - http_request.authorize(connection=self) - return http_request.headers - - try: - import boto.auth # noqa: F401 - except ImportError: - _S3Connection = _v19_S3Connection - else: - _S3Connection = _v20_S3Connection - - return _S3Connection - - class S3DownloadHandler: def __init__(self, settings, *, crawler=None, aws_access_key_id=None, aws_secret_access_key=None, httpdownloadhandler=HTTPDownloadHandler, **kw): + if not is_botocore_available(): + raise NotConfigured('missing botocore library') + if not aws_access_key_id: aws_access_key_id = settings['AWS_ACCESS_KEY_ID'] if not aws_secret_access_key: @@ -51,23 +30,15 @@ class S3DownloadHandler: self.anon = kw.get('anon') self._signer = None - if is_botocore(): - import botocore.auth - import botocore.credentials - kw.pop('anon', None) - if kw: - raise TypeError(f'Unexpected keyword arguments: {kw}') - if not self.anon: - SignerCls = botocore.auth.AUTH_TYPE_MAPS['s3'] - self._signer = SignerCls(botocore.credentials.Credentials( - aws_access_key_id, aws_secret_access_key)) - else: - _S3Connection = _get_boto_connection() - try: - self.conn = _S3Connection( - aws_access_key_id, aws_secret_access_key, **kw) - except Exception as ex: - raise NotConfigured(str(ex)) + import botocore.auth + import botocore.credentials + kw.pop('anon', None) + if kw: + raise TypeError(f'Unexpected keyword arguments: {kw}') + if not self.anon: + SignerCls = botocore.auth.AUTH_TYPE_MAPS['s3'] + self._signer = SignerCls(botocore.credentials.Credentials( + aws_access_key_id, aws_secret_access_key)) _http_handler = create_instance( objcls=httpdownloadhandler, diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 980825499..9f712285f 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -19,7 +19,7 @@ from zope.interface import implementer, Interface from scrapy import signals from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning -from scrapy.utils.boto import is_botocore +from scrapy.utils.boto import is_botocore_available from scrapy.utils.conf import feed_complete_default_values_from_settings from scrapy.utils.ftp import ftp_store_file from scrapy.utils.log import failure_to_exc_info @@ -120,22 +120,19 @@ class S3FeedStorage(BlockingFeedStorage): def __init__(self, uri, access_key=None, secret_key=None, acl=None, *, feed_options=None): + if not is_botocore_available(): + raise NotConfigured('missing botocore library') u = urlparse(uri) self.bucketname = u.hostname self.access_key = u.username or access_key self.secret_key = u.password or secret_key - self.is_botocore = is_botocore() self.keyname = u.path[1:] # remove first "/" self.acl = acl - if self.is_botocore: - import botocore.session - session = botocore.session.get_session() - self.s3_client = session.create_client( - 's3', aws_access_key_id=self.access_key, - aws_secret_access_key=self.secret_key) - else: - import boto - self.connect_s3 = boto.connect_s3 + import botocore.session + session = botocore.session.get_session() + self.s3_client = session.create_client( + 's3', aws_access_key_id=self.access_key, + aws_secret_access_key=self.secret_key) if feed_options and feed_options.get('overwrite', True) is False: logger.warning('S3 does not support appending to files. To ' 'suppress this warning, remove the overwrite ' @@ -154,18 +151,10 @@ class S3FeedStorage(BlockingFeedStorage): def _store_in_thread(self, file): file.seek(0) - if self.is_botocore: - kwargs = {'ACL': self.acl} if self.acl else {} - self.s3_client.put_object( - Bucket=self.bucketname, Key=self.keyname, Body=file, - **kwargs) - else: - conn = self.connect_s3(self.access_key, self.secret_key) - bucket = conn.get_bucket(self.bucketname, validate=False) - key = bucket.new_key(self.keyname) - kwargs = {'policy': self.acl} if self.acl else {} - key.set_contents_from_file(file, **kwargs) - key.close() + kwargs = {'ACL': self.acl} if self.acl else {} + self.s3_client.put_object( + Bucket=self.bucketname, Key=self.keyname, Body=file, + **kwargs) file.close() diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 99a72aa70..13ecd4e6c 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -11,7 +11,6 @@ import os import time from collections import defaultdict from contextlib import suppress -from email.utils import mktime_tz, parsedate_tz from ftplib import FTP from io import BytesIO from urllib.parse import urlparse @@ -23,7 +22,7 @@ from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request from scrapy.pipelines.media import MediaPipeline from scrapy.settings import Settings -from scrapy.utils.boto import is_botocore +from scrapy.utils.boto import is_botocore_available from scrapy.utils.datatypes import CaselessDict from scrapy.utils.ftp import ftp_store_file from scrapy.utils.log import failure_to_exc_info @@ -91,86 +90,54 @@ class S3FilesStore: } def __init__(self, uri): - self.is_botocore = is_botocore() - if self.is_botocore: - import botocore.session - session = botocore.session.get_session() - self.s3_client = session.create_client( - 's3', - aws_access_key_id=self.AWS_ACCESS_KEY_ID, - aws_secret_access_key=self.AWS_SECRET_ACCESS_KEY, - endpoint_url=self.AWS_ENDPOINT_URL, - region_name=self.AWS_REGION_NAME, - use_ssl=self.AWS_USE_SSL, - verify=self.AWS_VERIFY - ) - else: - from boto.s3.connection import S3Connection - self.S3Connection = S3Connection + if not is_botocore_available(): + raise NotConfigured('missing botocore library') + import botocore.session + session = botocore.session.get_session() + self.s3_client = session.create_client( + 's3', + aws_access_key_id=self.AWS_ACCESS_KEY_ID, + aws_secret_access_key=self.AWS_SECRET_ACCESS_KEY, + endpoint_url=self.AWS_ENDPOINT_URL, + region_name=self.AWS_REGION_NAME, + use_ssl=self.AWS_USE_SSL, + verify=self.AWS_VERIFY + ) if not uri.startswith("s3://"): raise ValueError(f"Incorrect URI scheme in {uri}, expected 's3'") self.bucket, self.prefix = uri[5:].split('/', 1) def stat_file(self, path, info): def _onsuccess(boto_key): - if self.is_botocore: - checksum = boto_key['ETag'].strip('"') - last_modified = boto_key['LastModified'] - modified_stamp = time.mktime(last_modified.timetuple()) - else: - checksum = boto_key.etag.strip('"') - last_modified = boto_key.last_modified - modified_tuple = parsedate_tz(last_modified) - modified_stamp = int(mktime_tz(modified_tuple)) + checksum = boto_key['ETag'].strip('"') + last_modified = boto_key['LastModified'] + modified_stamp = time.mktime(last_modified.timetuple()) return {'checksum': checksum, 'last_modified': modified_stamp} return self._get_boto_key(path).addCallback(_onsuccess) - def _get_boto_bucket(self): - # disable ssl (is_secure=False) because of this python bug: - # https://bugs.python.org/issue5103 - c = self.S3Connection(self.AWS_ACCESS_KEY_ID, self.AWS_SECRET_ACCESS_KEY, is_secure=False) - return c.get_bucket(self.bucket, validate=False) - def _get_boto_key(self, path): key_name = f'{self.prefix}{path}' - if self.is_botocore: - return threads.deferToThread( - self.s3_client.head_object, - Bucket=self.bucket, - Key=key_name) - else: - b = self._get_boto_bucket() - return threads.deferToThread(b.get_key, key_name) + return threads.deferToThread( + self.s3_client.head_object, + Bucket=self.bucket, + Key=key_name) def persist_file(self, path, buf, info, meta=None, headers=None): """Upload file to S3 storage""" key_name = f'{self.prefix}{path}' buf.seek(0) - if self.is_botocore: - extra = self._headers_to_botocore_kwargs(self.HEADERS) - if headers: - extra.update(self._headers_to_botocore_kwargs(headers)) - return threads.deferToThread( - self.s3_client.put_object, - Bucket=self.bucket, - Key=key_name, - Body=buf, - Metadata={k: str(v) for k, v in (meta or {}).items()}, - ACL=self.POLICY, - **extra) - else: - b = self._get_boto_bucket() - k = b.new_key(key_name) - if meta: - for metakey, metavalue in meta.items(): - k.set_metadata(metakey, str(metavalue)) - h = self.HEADERS.copy() - if headers: - h.update(headers) - return threads.deferToThread( - k.set_contents_from_string, buf.getvalue(), - headers=h, policy=self.POLICY) + extra = self._headers_to_botocore_kwargs(self.HEADERS) + if headers: + extra.update(self._headers_to_botocore_kwargs(headers)) + return threads.deferToThread( + self.s3_client.put_object, + Bucket=self.bucket, + Key=key_name, + Body=buf, + Metadata={k: str(v) for k, v in (meta or {}).items()}, + ACL=self.POLICY, + **extra) def _headers_to_botocore_kwargs(self, headers): """ Convert headers to botocore keyword agruments. diff --git a/scrapy/utils/boto.py b/scrapy/utils/boto.py index 12321caa5..3374c57c7 100644 --- a/scrapy/utils/boto.py +++ b/scrapy/utils/boto.py @@ -1,11 +1,32 @@ """Boto/botocore helpers""" +import warnings -from scrapy.exceptions import NotConfigured +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning def is_botocore(): + """ Returns True if botocore is available, otherwise raises NotConfigured. Never returns False. + + Previously, when boto was supported in addition to botocore, this returned False if boto was available + but botocore wasn't. + """ + message = ( + 'is_botocore() is deprecated and always returns True or raises an Exception, ' + 'so it cannot be used for checking if boto is available instead of botocore. ' + 'You can use scrapy.utils.boto.is_botocore_available() to check if botocore ' + 'is available.' + ) + warnings.warn(message, ScrapyDeprecationWarning, stacklevel=2) try: import botocore # noqa: F401 return True except ImportError: raise NotConfigured('missing botocore library') + + +def is_botocore_available(): + try: + import botocore # noqa: F401 + return True + except ImportError: + return False diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index f54942ffb..94d0ae2d3 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -10,8 +10,7 @@ from unittest import mock from importlib import import_module from twisted.trial.unittest import SkipTest -from scrapy.exceptions import NotConfigured -from scrapy.utils.boto import is_botocore +from scrapy.utils.boto import is_botocore_available def assert_aws_environ(): @@ -29,29 +28,19 @@ def assert_gcs_environ(): def skip_if_no_boto(): - try: - is_botocore() - except NotConfigured as e: - raise SkipTest(e) + if not is_botocore_available(): + raise SkipTest('missing botocore library') def get_s3_content_and_delete(bucket, path, with_key=False): """ Get content from s3 key, and delete key afterwards. """ - if is_botocore(): - import botocore.session - session = botocore.session.get_session() - client = session.create_client('s3') - key = client.get_object(Bucket=bucket, Key=path) - content = key['Body'].read() - client.delete_object(Bucket=bucket, Key=path) - else: - import boto - # assuming boto=2.2.2 - bucket = boto.connect_s3().get_bucket(bucket, validate=False) - key = bucket.get_key(path) - content = key.get_contents_as_string() - bucket.delete_key(path) + import botocore.session + session = botocore.session.get_session() + client = session.create_client('s3') + key = client.get_object(Bucket=bucket, Key=path) + content = key['Body'].read() + client.delete_object(Bucket=bucket, Key=path) return (content, key) if with_key else content diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 840e0f87b..33ac51712 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -45,6 +45,7 @@ from scrapy.utils.test import ( get_s3_content_and_delete, get_crawler, mock_google_cloud_storage, + skip_if_no_boto, ) from tests.mockserver import MockFTPServer, MockServer @@ -227,10 +228,7 @@ class BlockingFeedStorageTest(unittest.TestCase): class S3FeedStorageTest(unittest.TestCase): def test_parse_credentials(self): - try: - import botocore # noqa: F401 - except ImportError: - raise unittest.SkipTest("S3FeedStorage requires botocore") + skip_if_no_boto() aws_credentials = {'AWS_ACCESS_KEY_ID': 'settings_key', 'AWS_SECRET_ACCESS_KEY': 'settings_secret'} crawler = get_crawler(settings_dict=aws_credentials) @@ -324,11 +322,7 @@ class S3FeedStorageTest(unittest.TestCase): @defer.inlineCallbacks def test_store_botocore_without_acl(self): - try: - import botocore # noqa: F401 - except ImportError: - raise unittest.SkipTest('botocore is required') - + skip_if_no_boto() storage = S3FeedStorage( 's3://mybucket/export.csv', 'access_key', @@ -344,11 +338,7 @@ class S3FeedStorageTest(unittest.TestCase): @defer.inlineCallbacks def test_store_botocore_with_acl(self): - try: - import botocore # noqa: F401 - except ImportError: - raise unittest.SkipTest('botocore is required') - + skip_if_no_boto() storage = S3FeedStorage( 's3://mybucket/export.csv', 'access_key', @@ -366,57 +356,6 @@ class S3FeedStorageTest(unittest.TestCase): 'custom-acl' ) - @defer.inlineCallbacks - def test_store_not_botocore_without_acl(self): - storage = S3FeedStorage( - 's3://mybucket/export.csv', - 'access_key', - 'secret_key', - ) - self.assertEqual(storage.access_key, 'access_key') - self.assertEqual(storage.secret_key, 'secret_key') - self.assertEqual(storage.acl, None) - - storage.is_botocore = False - storage.connect_s3 = mock.MagicMock() - self.assertFalse(storage.is_botocore) - - yield storage.store(BytesIO(b'test file')) - - conn = storage.connect_s3(*storage.connect_s3.call_args) - bucket = conn.get_bucket(*conn.get_bucket.call_args) - key = bucket.new_key(*bucket.new_key.call_args) - self.assertNotIn( - dict(policy='custom-acl'), - key.set_contents_from_file.call_args - ) - - @defer.inlineCallbacks - def test_store_not_botocore_with_acl(self): - storage = S3FeedStorage( - 's3://mybucket/export.csv', - 'access_key', - 'secret_key', - 'custom-acl' - ) - self.assertEqual(storage.access_key, 'access_key') - self.assertEqual(storage.secret_key, 'secret_key') - self.assertEqual(storage.acl, 'custom-acl') - - storage.is_botocore = False - storage.connect_s3 = mock.MagicMock() - self.assertFalse(storage.is_botocore) - - yield storage.store(BytesIO(b'test file')) - - conn = storage.connect_s3(*storage.connect_s3.call_args) - bucket = conn.get_bucket(*conn.get_bucket.call_args) - key = bucket.new_key(*bucket.new_key.call_args) - self.assertIn( - dict(policy='custom-acl'), - key.set_contents_from_file.call_args - ) - def test_overwrite_default(self): with LogCapture() as log: S3FeedStorage( diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 1dd7031fe..d5b0bb3d8 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -22,7 +22,6 @@ from scrapy.pipelines.files import ( S3FilesStore, ) from scrapy.settings import Settings -from scrapy.utils.boto import is_botocore from scrapy.utils.test import ( assert_aws_environ, assert_gcs_environ, @@ -437,16 +436,10 @@ class TestS3FilesStore(unittest.TestCase): content, key = get_s3_content_and_delete( u.hostname, u.path[1:], with_key=True) self.assertEqual(content, data) - if is_botocore(): - self.assertEqual(key['Metadata'], {'foo': 'bar'}) - self.assertEqual( - key['CacheControl'], S3FilesStore.HEADERS['Cache-Control']) - self.assertEqual(key['ContentType'], 'image/png') - else: - self.assertEqual(key.metadata, {'foo': 'bar'}) - self.assertEqual( - key.cache_control, S3FilesStore.HEADERS['Cache-Control']) - self.assertEqual(key.content_type, 'image/png') + self.assertEqual(key['Metadata'], {'foo': 'bar'}) + self.assertEqual( + key['CacheControl'], S3FilesStore.HEADERS['Cache-Control']) + self.assertEqual(key['ContentType'], 'image/png') class TestGCSFilesStore(unittest.TestCase): From 7f1e74daa28caa481b4a484392a7dd2a18fc290e Mon Sep 17 00:00:00 2001 From: Mirwaisse Djanbaz Date: Mon, 21 Sep 2020 14:38:16 +0200 Subject: [PATCH 15/34] =?UTF-8?q?dependencides=20=E2=86=92=20dependencies?= =?UTF-8?q?=20(#4800)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/intro/install.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/intro/install.rst b/docs/intro/install.rst index 8b4240bf6..fe7bc0a2a 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -211,7 +211,7 @@ PyPy We recommend using the latest PyPy version. The version tested is 5.9.0. For PyPy3, only Linux installation was tested. -Most Scrapy dependencides now have binary wheels for CPython, but not for PyPy. +Most Scrapy dependencies now have binary wheels for CPython, but not for PyPy. This means that these dependecies will be built during installation. On macOS, you are likely to face an issue with building Cryptography dependency, solution to this problem is described From 3989f64baa39f7e42b0f798dec15cd250e0fba21 Mon Sep 17 00:00:00 2001 From: Mirwaisse Djanbaz Date: Mon, 21 Sep 2020 14:40:00 +0200 Subject: [PATCH 16/34] =?UTF-8?q?dependecies=20=E2=86=92=20dependencies=20?= =?UTF-8?q?(#4801)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/intro/install.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/intro/install.rst b/docs/intro/install.rst index fe7bc0a2a..3bfd3bc3b 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -212,7 +212,7 @@ We recommend using the latest PyPy version. The version tested is 5.9.0. For PyPy3, only Linux installation was tested. Most Scrapy dependencies now have binary wheels for CPython, but not for PyPy. -This means that these dependecies will be built during installation. +This means that these dependencies will be built during installation. On macOS, you are likely to face an issue with building Cryptography dependency, solution to this problem is described `here `_, From 008cf1c75ebe72a607149d7efd8f902f8763bc52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 21 Sep 2020 20:45:21 +0200 Subject: [PATCH 17/34] Remove a test that has never been executed in Python 3 --- tests/test_downloader_handlers.py | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 0a374c161..3e8d7e6b9 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -868,29 +868,6 @@ class S3TestCase(unittest.TestCase): self.assertEqual(httpreq.headers['Authorization'], b'AWS 0PN5J17HBGZHT7JJ3X82:thdUi9VAkzhkniLj96JIrOPGi0g=') - def test_request_signing5(self): - try: - import botocore # noqa: F401 - except ImportError: - pass - else: - raise unittest.SkipTest( - 'botocore does not support overriding date with x-amz-date') - # deletes an object from the 'johnsmith' bucket using the - # path-style and Date alternative. - date = 'Tue, 27 Mar 2007 21:20:27 +0000' - req = Request( - 's3://johnsmith/photos/puppy.jpg', method='DELETE', headers={ - 'Date': date, - 'x-amz-date': 'Tue, 27 Mar 2007 21:20:26 +0000', - }) - with self._mocked_date(date): - httpreq = self.download_request(req, self.spider) - # botocore does not override Date with x-amz-date - self.assertEqual( - httpreq.headers['Authorization'], - b'AWS 0PN5J17HBGZHT7JJ3X82:k3nL7gH3+PadhTEVn5Ip83xlYzk=') - def test_request_signing6(self): # uploads an object to a CNAME style virtual hosted bucket with metadata. date = 'Tue, 27 Mar 2007 21:06:08 +0000' From 56f05fb16476b40f773edc9e93d41c8893a349c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 21 Sep 2020 22:01:09 +0200 Subject: [PATCH 18/34] Use mocking for tests/test_feedexport.py::S3FeedStorageTest::test_store --- tests/test_feedexport.py | 50 +++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 33ac51712..a8c0cf686 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -13,6 +13,7 @@ from logging import getLogger from pathlib import Path from string import ascii_letters, digits from unittest import mock +from unittest.mock import call from urllib.parse import urljoin, urlparse, quote from urllib.request import pathname2url @@ -254,21 +255,42 @@ class S3FeedStorageTest(unittest.TestCase): @defer.inlineCallbacks def test_store(self): - assert_aws_environ() - uri = os.environ.get('S3_TEST_FILE_URI') - if not uri: - raise unittest.SkipTest("No S3 URI available for testing") - access_key = os.environ.get('AWS_ACCESS_KEY_ID') - secret_key = os.environ.get('AWS_SECRET_ACCESS_KEY') - storage = S3FeedStorage(uri, access_key, secret_key) + skip_if_no_boto() + + settings = { + 'AWS_ACCESS_KEY_ID': 'access_key', + 'AWS_SECRET_ACCESS_KEY': 'secret_key', + } + crawler = get_crawler(settings_dict=settings) + bucket = 'mybucket' + key = 'export.csv' + storage = S3FeedStorage.from_crawler(crawler, f's3://{bucket}/{key}') verifyObject(IFeedStorage, storage) - file = storage.open(scrapy.Spider("default")) - expected_content = b"content: \xe2\x98\x83" - file.write(expected_content) - yield storage.store(file) - u = urlparse(uri) - content = get_s3_content_and_delete(u.hostname, u.path[1:]) - self.assertEqual(content, expected_content) + + file = mock.MagicMock() + from botocore.stub import Stubber + with Stubber(storage.s3_client) as stub: + stub.add_response( + 'put_object', + expected_params={ + 'Body': file, + 'Bucket': bucket, + 'Key': key, + }, + service_response={}, + ) + + yield storage.store(file) + + stub.assert_no_pending_responses() + self.assertEqual( + file.method_calls, + [ + call.seek(0), + # The call to read does not happen with Stubber + call.close(), + ] + ) def test_init_without_acl(self): storage = S3FeedStorage( From 17e135377a564152e82cd2ad177a6a2749c15832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 21 Sep 2020 22:54:39 +0200 Subject: [PATCH 19/34] Use mocking for tests/test_feedexport.py::BatchDeliveriesTest::test_s3_export --- tests/test_feedexport.py | 99 +++++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 51 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index a8c0cf686..ac0b3ccaa 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -13,7 +13,6 @@ from logging import getLogger from pathlib import Path from string import ascii_letters, digits from unittest import mock -from unittest.mock import call from urllib.parse import urljoin, urlparse, quote from urllib.request import pathname2url @@ -42,7 +41,6 @@ from scrapy.extensions.feedexport import ( from scrapy.settings import Settings from scrapy.utils.python import to_unicode from scrapy.utils.test import ( - assert_aws_environ, get_s3_content_and_delete, get_crawler, mock_google_cloud_storage, @@ -286,9 +284,9 @@ class S3FeedStorageTest(unittest.TestCase): self.assertEqual( file.method_calls, [ - call.seek(0), + mock.call.seek(0), # The call to read does not happen with Stubber - call.close(), + mock.call.close(), ] ) @@ -1518,46 +1516,53 @@ class BatchDeliveriesTest(FeedExportTestBase): @defer.inlineCallbacks def test_s3_export(self): - """ - Test export of items into s3 bucket. - S3_TEST_BUCKET_NAME, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY must be specified in tox.ini - to perform this test: - [testenv] - setenv = - AWS_SECRET_ACCESS_KEY = ABCD - AWS_ACCESS_KEY_ID = EFGH - S3_TEST_BUCKET_NAME = IJKL - """ - try: - import boto3 - except ImportError: - raise unittest.SkipTest("S3FeedStorage requires boto3") + skip_if_no_boto() - assert_aws_environ() - s3_test_bucket_name = os.environ.get('S3_TEST_BUCKET_NAME') - access_key = os.environ.get('AWS_ACCESS_KEY_ID') - secret_key = os.environ.get('AWS_SECRET_ACCESS_KEY') - if not s3_test_bucket_name: - raise unittest.SkipTest("No S3 BUCKET available for testing") - - chars = [random.choice(ascii_letters + digits) for _ in range(15)] - filename = ''.join(chars) - prefix = f'tmp/{filename}' - s3_test_file_uri = f's3://{s3_test_bucket_name}/{prefix}/%(batch_time)s.json' - storage = S3FeedStorage(s3_test_bucket_name, access_key, secret_key) - settings = Settings({ - 'FEEDS': { - s3_test_file_uri: { - 'format': 'json', - }, - }, - 'FEED_EXPORT_BATCH_ITEM_COUNT': 1, - }) + bucket = 'mybucket' items = [ self.MyItem({'foo': 'bar1', 'egg': 'spam1'}), self.MyItem({'foo': 'bar2', 'egg': 'spam2', 'baz': 'quux2'}), self.MyItem({'foo': 'bar3', 'baz': 'quux3'}), ] + + class CustomS3FeedStorage(S3FeedStorage): + + stubs = [] + + def open(self, *args, **kwargs): + from botocore.stub import ANY, Stubber + stub = Stubber(self.s3_client) + stub.activate() + CustomS3FeedStorage.stubs.append(stub) + stub.add_response( + 'put_object', + expected_params={ + 'Body': ANY, + 'Bucket': bucket, + 'Key': ANY, + }, + service_response={}, + ) + return super().open(*args, **kwargs) + + key = 'export.csv' + uri = f's3://{bucket}/{key}/%(batch_time)s.json' + batch_item_count = 1 + settings = { + 'AWS_ACCESS_KEY_ID': 'access_key', + 'AWS_SECRET_ACCESS_KEY': 'secret_key', + 'FEED_EXPORT_BATCH_ITEM_COUNT': batch_item_count, + 'FEED_STORAGES': { + 's3': CustomS3FeedStorage, + }, + 'FEEDS': { + uri: { + 'format': 'json', + }, + }, + } + crawler = get_crawler(settings_dict=settings) + storage = S3FeedStorage.from_crawler(crawler, uri) verifyObject(IFeedStorage, storage) class TestSpider(scrapy.Spider): @@ -1567,22 +1572,14 @@ class BatchDeliveriesTest(FeedExportTestBase): for item in items: yield item - s3 = boto3.resource('s3') - my_bucket = s3.Bucket(s3_test_bucket_name) - batch_size = settings.getint('FEED_EXPORT_BATCH_ITEM_COUNT') - - with MockServer() as s: + with MockServer() as server: runner = CrawlerRunner(Settings(settings)) - TestSpider.start_urls = [s.url('/')] + TestSpider.start_urls = [server.url('/')] yield runner.crawl(TestSpider) - for file_uri in my_bucket.objects.filter(Prefix=prefix): - content = get_s3_content_and_delete(s3_test_bucket_name, file_uri.key) - if not content and not items: - break - content = json.loads(content.decode('utf-8')) - expected_batch, items = items[:batch_size], items[batch_size:] - self.assertEqual(expected_batch, content) + self.assertEqual(len(CustomS3FeedStorage.stubs), len(items)+1) + for stub in CustomS3FeedStorage.stubs[:-1]: + stub.assert_no_pending_responses() class FeedExportInitTest(unittest.TestCase): From 35726da434a9bc61dcb1ad5ef475c7d030023c87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 21 Sep 2020 22:55:25 +0200 Subject: [PATCH 20/34] tests/test_feedexport.py: remove unused import --- tests/test_feedexport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index ac0b3ccaa..18f7f8458 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -13,7 +13,7 @@ from logging import getLogger from pathlib import Path from string import ascii_letters, digits from unittest import mock -from urllib.parse import urljoin, urlparse, quote +from urllib.parse import urljoin, quote from urllib.request import pathname2url import lxml.etree From c3b740f07814107e643bb0d073ea116049e37cba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 21 Sep 2020 23:25:37 +0200 Subject: [PATCH 21/34] Use mocking for tests/test_pipeline_files.py::TestS3FilesStore::test_persist --- scrapy/utils/test.py | 9 ---- tests/test_pipeline_files.py | 100 +++++++++++++++++++++++++++-------- 2 files changed, 78 insertions(+), 31 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 94d0ae2d3..cf251442f 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -13,15 +13,6 @@ from twisted.trial.unittest import SkipTest from scrapy.utils.boto import is_botocore_available -def assert_aws_environ(): - """Asserts the current environment is suitable for running AWS testsi. - Raises SkipTest with the reason if it's not. - """ - skip_if_no_boto() - if 'AWS_ACCESS_KEY_ID' not in os.environ: - raise SkipTest("AWS keys not found") - - def assert_gcs_environ(): if 'GCS_PROJECT_ID' not in os.environ: raise SkipTest("GCS_PROJECT_ID not found") diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index d5b0bb3d8..1b14e3b1e 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -1,6 +1,7 @@ import os import random import time +from datetime import datetime from io import BytesIO from shutil import rmtree from tempfile import mkdtemp @@ -23,11 +24,11 @@ from scrapy.pipelines.files import ( ) from scrapy.settings import Settings from scrapy.utils.test import ( - assert_aws_environ, assert_gcs_environ, get_ftp_content_and_delete, get_gcs_content_and_delete, get_s3_content_and_delete, + skip_if_no_boto, ) @@ -414,32 +415,87 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): class TestS3FilesStore(unittest.TestCase): + @defer.inlineCallbacks def test_persist(self): - assert_aws_environ() - uri = os.environ.get('S3_TEST_FILE_URI') - if not uri: - raise unittest.SkipTest("No S3 URI available for testing") - data = b"TestS3FilesStore: \xe2\x98\x83" - buf = BytesIO(data) + skip_if_no_boto() + + bucket = 'mybucket' + key = 'export.csv' + uri = f's3://{bucket}/{key}' + buffer = mock.MagicMock() meta = {'foo': 'bar'} path = '' + content_type = 'image/png' + store = S3FilesStore(uri) - yield store.persist_file( - path, buf, info=None, meta=meta, - headers={'Content-Type': 'image/png'}) - s = yield store.stat_file(path, info=None) - self.assertIn('last_modified', s) - self.assertIn('checksum', s) - self.assertEqual(s['checksum'], '3187896a9657a28163abb31667df64c8') - u = urlparse(uri) - content, key = get_s3_content_and_delete( - u.hostname, u.path[1:], with_key=True) - self.assertEqual(content, data) - self.assertEqual(key['Metadata'], {'foo': 'bar'}) - self.assertEqual( - key['CacheControl'], S3FilesStore.HEADERS['Cache-Control']) - self.assertEqual(key['ContentType'], 'image/png') + from botocore.stub import Stubber + with Stubber(store.s3_client) as stub: + stub.add_response( + 'put_object', + expected_params={ + 'ACL': S3FilesStore.POLICY, + 'Body': buffer, + 'Bucket': bucket, + 'CacheControl': S3FilesStore.HEADERS['Cache-Control'], + 'ContentType': content_type, + 'Key': key, + 'Metadata': meta, + }, + service_response={}, + ) + + yield store.persist_file( + path, + buffer, + info=None, + meta=meta, + headers={'Content-Type': content_type}, + ) + + stub.assert_no_pending_responses() + self.assertEqual( + buffer.method_calls, + [ + mock.call.seek(0), + # The call to read does not happen with Stubber + ] + ) + + @defer.inlineCallbacks + def test_stat(self): + skip_if_no_boto() + + bucket = 'mybucket' + key = 'export.csv' + uri = f's3://{bucket}/{key}' + checksum = '3187896a9657a28163abb31667df64c8' + + store = S3FilesStore(uri) + from botocore.stub import Stubber + with Stubber(store.s3_client) as stub: + stub.add_response( + 'head_object', + expected_params={ + 'Bucket': bucket, + 'Key': key, + }, + service_response={ + 'ETag': f'"{checksum}"', + 'LastModified': datetime(2019, 12, 1), + }, + ) + + file_stats = yield store.stat_file('', info=None) + self.assertEqual( + file_stats, + { + 'checksum': checksum, + 'last_modified': 1575154800, + }, + ) + + stub.assert_no_pending_responses() class TestGCSFilesStore(unittest.TestCase): From 8f46e845190b7affb7a02c6842b465f0c5c3b76c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 21 Sep 2020 23:28:16 +0200 Subject: [PATCH 22/34] Fix style issues --- scrapy/utils/test.py | 12 ------------ tests/test_feedexport.py | 3 +-- tests/test_pipeline_files.py | 1 - 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index cf251442f..24c38283a 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -23,18 +23,6 @@ def skip_if_no_boto(): raise SkipTest('missing botocore library') -def get_s3_content_and_delete(bucket, path, with_key=False): - """ Get content from s3 key, and delete key afterwards. - """ - import botocore.session - session = botocore.session.get_session() - client = session.create_client('s3') - key = client.get_object(Bucket=bucket, Key=path) - content = key['Body'].read() - client.delete_object(Bucket=bucket, Key=path) - return (content, key) if with_key else content - - def get_gcs_content_and_delete(bucket, path): from google.cloud import storage client = storage.Client(project=os.environ.get('GCS_PROJECT_ID')) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 18f7f8458..3ed35bec5 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -41,7 +41,6 @@ from scrapy.extensions.feedexport import ( from scrapy.settings import Settings from scrapy.utils.python import to_unicode from scrapy.utils.test import ( - get_s3_content_and_delete, get_crawler, mock_google_cloud_storage, skip_if_no_boto, @@ -1577,7 +1576,7 @@ class BatchDeliveriesTest(FeedExportTestBase): TestSpider.start_urls = [server.url('/')] yield runner.crawl(TestSpider) - self.assertEqual(len(CustomS3FeedStorage.stubs), len(items)+1) + self.assertEqual(len(CustomS3FeedStorage.stubs), len(items) + 1) for stub in CustomS3FeedStorage.stubs[:-1]: stub.assert_no_pending_responses() diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 1b14e3b1e..e840298d9 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -27,7 +27,6 @@ from scrapy.utils.test import ( assert_gcs_environ, get_ftp_content_and_delete, get_gcs_content_and_delete, - get_s3_content_and_delete, skip_if_no_boto, ) From 07c1d9c25b6f499fbe2cc0f0139d78cf6c1d204a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 21 Sep 2020 23:32:55 +0200 Subject: [PATCH 23/34] Remove boto3 as a dependency for tests --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index 12e40295c..ea356c56a 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,6 @@ deps = -ctests/constraints.txt -rtests/requirements-py3.txt # Extras - boto3>=1.13.0 botocore>=1.4.87 Pillow>=4.0.0 passenv = From 6ef7c44061f74281e7a977c9b1de4892cf84caf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 22 Sep 2020 12:45:21 +0200 Subject: [PATCH 24/34] Fix timezone test issue --- tests/test_pipeline_files.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index e840298d9..4e1b90787 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -469,6 +469,7 @@ class TestS3FilesStore(unittest.TestCase): key = 'export.csv' uri = f's3://{bucket}/{key}' checksum = '3187896a9657a28163abb31667df64c8' + last_modified = datetime(2019, 12, 1) store = S3FilesStore(uri) from botocore.stub import Stubber @@ -481,7 +482,7 @@ class TestS3FilesStore(unittest.TestCase): }, service_response={ 'ETag': f'"{checksum}"', - 'LastModified': datetime(2019, 12, 1), + 'LastModified': last_modified, }, ) @@ -490,7 +491,7 @@ class TestS3FilesStore(unittest.TestCase): file_stats, { 'checksum': checksum, - 'last_modified': 1575154800, + 'last_modified': last_modified.timestamp(), }, ) From eff96038c7a488860c1c58c2c7e37f888264c3dc Mon Sep 17 00:00:00 2001 From: madeny <7504281+madeny@users.noreply.github.com> Date: Sat, 26 Sep 2020 22:50:38 +0200 Subject: [PATCH 25/34] Correct some typos This won't be an issue if **your** spider doesn't rely on cookies. --- docs/topics/jobs.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/jobs.rst b/docs/topics/jobs.rst index 58601824a..d855d0133 100644 --- a/docs/topics/jobs.rst +++ b/docs/topics/jobs.rst @@ -65,7 +65,7 @@ Cookies expiration ------------------ Cookies may expire. So, if you don't resume your spider quickly the requests -scheduled may no longer work. This won't be an issue if you spider doesn't rely +scheduled may no longer work. This won't be an issue if your spider doesn't rely on cookies. From 894b509d7a4262cfc7548d78e9ff4831c6551c34 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Tue, 29 Sep 2020 23:37:28 -0300 Subject: [PATCH 26/34] Crawl rule: remove deprecated code Remove the compatibility layer that handles 'process_request' methods that do not receive a 'response' parameter --- scrapy/spiders/crawl.py | 34 ++++++++++--------------------- tests/test_spider.py | 45 +++++++++++++++++------------------------ 2 files changed, 30 insertions(+), 49 deletions(-) diff --git a/scrapy/spiders/crawl.py b/scrapy/spiders/crawl.py index bc4551a54..1dcf2e6ab 100644 --- a/scrapy/spiders/crawl.py +++ b/scrapy/spiders/crawl.py @@ -6,14 +6,11 @@ See documentation in docs/topics/spiders.rst """ import copy -import warnings from typing import Sequence -from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, HtmlResponse from scrapy.linkextractors import LinkExtractor from scrapy.spiders import Spider -from scrapy.utils.python import get_func_args from scrapy.utils.spider import iterate_spider_output @@ -37,15 +34,22 @@ _default_link_extractor = LinkExtractor() class Rule: - def __init__(self, link_extractor=None, callback=None, cb_kwargs=None, follow=None, - process_links=None, process_request=None, errback=None): + def __init__( + self, + link_extractor=None, + callback=None, + cb_kwargs=None, + follow=None, + process_links=None, + process_request=None, + errback=None, + ): self.link_extractor = link_extractor or _default_link_extractor self.callback = callback self.errback = errback self.cb_kwargs = cb_kwargs or {} self.process_links = process_links or _identity self.process_request = process_request or _identity_process_request - self.process_request_argcount = None self.follow = follow if follow is not None else not callback def _compile(self, spider): @@ -53,22 +57,6 @@ class Rule: self.errback = _get_method(self.errback, spider) self.process_links = _get_method(self.process_links, spider) self.process_request = _get_method(self.process_request, spider) - self.process_request_argcount = len(get_func_args(self.process_request)) - if self.process_request_argcount == 1: - warnings.warn( - "Rule.process_request should accept two arguments " - "(request, response), accepting only one is deprecated", - category=ScrapyDeprecationWarning, - stacklevel=2, - ) - - def _process_request(self, request, response): - """ - Wrapper around the request processing function to maintain backward - compatibility with functions that do not take a Response object - """ - args = [request] if self.process_request_argcount == 1 else [request, response] - return self.process_request(*args) class CrawlSpider(Spider): @@ -111,7 +99,7 @@ class CrawlSpider(Spider): for link in rule.process_links(links): seen.add(link) request = self._build_request(rule_index, link) - yield rule._process_request(request, response) + yield rule.process_request(request, response) def _callback(self, response): rule = self._rules[response.meta['rule']] diff --git a/tests/test_spider.py b/tests/test_spider.py index 78157a9b9..d23543f6a 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -1,8 +1,8 @@ import gzip import inspect -from unittest import mock import warnings from io import BytesIO +from unittest import mock from testfixtures import LogCapture from twisted.trial import unittest @@ -20,7 +20,6 @@ from scrapy.spiders import ( XMLFeedSpider, ) from scrapy.linkextractors import LinkExtractor -from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.test import get_crawler @@ -280,7 +279,7 @@ class CrawlSpiderTest(SpiderTest): response = HtmlResponse("http://example.org/somepage/index.html", body=self.test_body) - def process_request_change_domain(request): + def process_request_change_domain(request, response): return request.replace(url=request.url.replace('.org', '.com')) class _CrawlSpider(self.spider_class): @@ -290,17 +289,14 @@ class CrawlSpiderTest(SpiderTest): Rule(LinkExtractor(), process_request=process_request_change_domain), ) - with warnings.catch_warnings(record=True) as cw: - spider = _CrawlSpider() - output = list(spider._requests_to_follow(response)) - self.assertEqual(len(output), 3) - self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) - self.assertEqual([r.url for r in output], - ['http://example.com/somepage/item/12.html', - 'http://example.com/about.html', - 'http://example.com/nofollow.html']) - self.assertEqual(len(cw), 1) - self.assertEqual(cw[0].category, ScrapyDeprecationWarning) + spider = _CrawlSpider() + output = list(spider._requests_to_follow(response)) + self.assertEqual(len(output), 3) + self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) + self.assertEqual([r.url for r in output], + ['http://example.com/somepage/item/12.html', + 'http://example.com/about.html', + 'http://example.com/nofollow.html']) def test_process_request_with_response(self): @@ -339,20 +335,17 @@ class CrawlSpiderTest(SpiderTest): Rule(LinkExtractor(), process_request='process_request_upper'), ) - def process_request_upper(self, request): + def process_request_upper(self, request, response): return request.replace(url=request.url.upper()) - with warnings.catch_warnings(record=True) as cw: - spider = _CrawlSpider() - output = list(spider._requests_to_follow(response)) - self.assertEqual(len(output), 3) - self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) - self.assertEqual([r.url for r in output], - ['http://EXAMPLE.ORG/SOMEPAGE/ITEM/12.HTML', - 'http://EXAMPLE.ORG/ABOUT.HTML', - 'http://EXAMPLE.ORG/NOFOLLOW.HTML']) - self.assertEqual(len(cw), 1) - self.assertEqual(cw[0].category, ScrapyDeprecationWarning) + spider = _CrawlSpider() + output = list(spider._requests_to_follow(response)) + self.assertEqual(len(output), 3) + self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) + self.assertEqual([r.url for r in output], + ['http://EXAMPLE.ORG/SOMEPAGE/ITEM/12.HTML', + 'http://EXAMPLE.ORG/ABOUT.HTML', + 'http://EXAMPLE.ORG/NOFOLLOW.HTML']) def test_process_request_instance_method_with_response(self): From 9661a8dcfc79249e8e3e117eb70682ebc8e57a92 Mon Sep 17 00:00:00 2001 From: Sashreek Shankar <45600974+sashreek1@users.noreply.github.com> Date: Thu, 1 Oct 2020 06:46:12 +0530 Subject: [PATCH 27/34] removed datatype specification for *args & ** kwargs --- scrapy/crawler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 4c6b0e496..578016536 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -180,9 +180,9 @@ class CrawlerRunner: :type crawler_or_spidercls: :class:`~scrapy.crawler.Crawler` instance, :class:`~scrapy.spiders.Spider` subclass or string - :param list args: arguments to initialize the spider + :param args: arguments to initialize the spider - :param dict kwargs: keyword arguments to initialize the spider + :param kwargs: keyword arguments to initialize the spider """ if isinstance(crawler_or_spidercls, Spider): raise ValueError( From 44f0fde9057256dc16f24f22cfebc61626a94450 Mon Sep 17 00:00:00 2001 From: Andrey Rahmatullin Date: Thu, 1 Oct 2020 23:21:09 +0500 Subject: [PATCH 28/34] Simplify TLS logging for the modern pyOpenSSL. (#4822) --- scrapy/core/downloader/tls.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index d9f3750d5..b5c6cc895 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -55,18 +55,11 @@ class ScrapyClientTLSOptions(ClientTLSOptions): set_tlsext_host_name(connection, self._hostnameBytes) elif where & SSL.SSL_CB_HANDSHAKE_DONE: if self.verbose_logging: - if hasattr(connection, 'get_cipher_name'): # requires pyOPenSSL 0.15 - if hasattr(connection, 'get_protocol_version_name'): # requires pyOPenSSL 16.0.0 - logger.debug('SSL connection to %s using protocol %s, cipher %s', - self._hostnameASCII, - connection.get_protocol_version_name(), - connection.get_cipher_name(), - ) - else: - logger.debug('SSL connection to %s using cipher %s', - self._hostnameASCII, - connection.get_cipher_name(), - ) + logger.debug('SSL connection to %s using protocol %s, cipher %s', + self._hostnameASCII, + connection.get_protocol_version_name(), + connection.get_cipher_name(), + ) server_cert = connection.get_peer_certificate() logger.debug('SSL connection certificate: issuer "%s", subject "%s"', x509name_to_string(server_cert.get_issuer()), From e42d82526a1a519ff1305b189acde064a40fbd8d Mon Sep 17 00:00:00 2001 From: Andrey Rahmatullin Date: Thu, 1 Oct 2020 23:22:17 +0500 Subject: [PATCH 29/34] Drop the conditional code for old Twisted (#4820) --- scrapy/core/downloader/tls.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index b5c6cc895..2b8990b75 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -5,7 +5,6 @@ from service_identity.exceptions import CertificateError from twisted.internet._sslverify import ClientTLSOptions, verifyHostname, VerificationError from twisted.internet.ssl import AcceptableCiphers -from scrapy import twisted_version from scrapy.utils.ssl import x509name_to_string, get_temp_key_info @@ -28,13 +27,6 @@ openssl_methods = { } -if twisted_version < (17, 0, 0): - from twisted.internet._sslverify import _maybeSetHostNameIndication as set_tlsext_host_name -else: - def set_tlsext_host_name(connection, hostNameBytes): - connection.set_tlsext_host_name(hostNameBytes) - - class ScrapyClientTLSOptions(ClientTLSOptions): """ SSL Client connection creator ignoring certificate verification errors @@ -52,7 +44,7 @@ class ScrapyClientTLSOptions(ClientTLSOptions): def _identityVerifyingInfoCallback(self, connection, where, ret): if where & SSL.SSL_CB_HANDSHAKE_START: - set_tlsext_host_name(connection, self._hostnameBytes) + connection.set_tlsext_host_name(self._hostnameBytes) elif where & SSL.SSL_CB_HANDSHAKE_DONE: if self.verbose_logging: logger.debug('SSL connection to %s using protocol %s, cipher %s', From f47b120e2b67be7821a06ff77786c0bab2cfece8 Mon Sep 17 00:00:00 2001 From: Habeeb Shopeju Date: Thu, 1 Oct 2020 19:50:11 +0100 Subject: [PATCH 30/34] Documentation of link extractor usage (#4775) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added description when using link extractor outside crawlspiders and created reference documentation for scrapy.link.Link class * Added link.rst to toctree * Corrected spelling errors, moved docs to Link doctstring to use autoclass * Moved link docs to link_extractors * Update docs/topics/link-extractors.rst Co-authored-by: Adrián Chaves * Update link.py Improvements to URL description * Update link.py * Update link.py Fixed line length Flake issue * Update link.py Fixed trailing whitespace Co-authored-by: Adrián Chaves --- docs/index.rst | 1 - docs/topics/link-extractors.rst | 21 ++++++++++++++++++--- scrapy/link.py | 17 ++++++++++++++++- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 11aa5c9be..da264fb34 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -78,7 +78,6 @@ Basic concepts topics/settings topics/exceptions - :doc:`topics/commands` Learn about the command-line tool used to manage your Scrapy project. diff --git a/docs/topics/link-extractors.rst b/docs/topics/link-extractors.rst index ed32411b0..e12ad45e0 100644 --- a/docs/topics/link-extractors.rst +++ b/docs/topics/link-extractors.rst @@ -10,12 +10,19 @@ The ``__init__`` method of :class:`~scrapy.linkextractors.lxmlhtml.LxmlLinkExtractor` takes settings that determine which links may be extracted. :class:`LxmlLinkExtractor.extract_links ` returns a -list of matching :class:`scrapy.link.Link` objects from a +list of matching :class:`~scrapy.link.Link` objects from a :class:`~scrapy.http.Response` object. Link extractors are used in :class:`~scrapy.spiders.CrawlSpider` spiders -through a set of :class:`~scrapy.spiders.Rule` objects. You can also use link -extractors in regular spiders. +through a set of :class:`~scrapy.spiders.Rule` objects. + +You can also use link extractors in regular spiders. For example, you can instantiate +:class:`LinkExtractor ` into a class +variable in your spider, and use it from your spider callbacks:: + + def parse(self, response): + for link in self.link_extractor.extract_links(response): + yield Request(link.url, callback=self.parse) .. _topics-link-extractors-ref: @@ -145,4 +152,12 @@ LxmlLinkExtractor .. automethod:: extract_links +Link +---- + +.. module:: scrapy.link + :synopsis: Link from link extractors + +.. autoclass:: Link + .. _scrapy.linkextractors: https://github.com/scrapy/scrapy/blob/master/scrapy/linkextractors/__init__.py diff --git a/scrapy/link.py b/scrapy/link.py index 684735f6e..e70667361 100644 --- a/scrapy/link.py +++ b/scrapy/link.py @@ -7,7 +7,22 @@ its documentation in: docs/topics/link-extractors.rst class Link: - """Link objects represent an extracted link by the LinkExtractor.""" + """Link objects represent an extracted link by the LinkExtractor. + + Using the anchor tag sample below to illustrate the parameters:: + + Dont follow this one + + :param url: the absolute url being linked to in the anchor tag. + From the sample, this is ``https://example.com/nofollow.html``. + + :param text: the text in the anchor tag. From the sample, this is ``Dont follow this one``. + + :param fragment: the part of the url after the hash symbol. From the sample, this is ``foo``. + + :param nofollow: an indication of the presence or absence of a nofollow value in the ``rel`` attribute + of the anchor tag. + """ __slots__ = ['url', 'text', 'fragment', 'nofollow'] From 159e2b2e2fbbcbb2b083a79e83e37cf4e60ee5ee Mon Sep 17 00:00:00 2001 From: Akshay Sharma <42249933+AKSHAYSHARMAJS@users.noreply.github.com> Date: Fri, 2 Oct 2020 00:23:08 +0530 Subject: [PATCH 31/34] allowing to run .pyw files (#4646) * allow .pyw in scrapy/commands/runspider.py * aesthetics * added tests for '.pyw' * created class for testing .pyw files * name=None parameter in get_log * small fix * .pyw tests for non-windows * used @skipIf for tests * two more tests skipped --- scrapy/commands/runspider.py | 2 +- tests/test_commands.py | 91 ++++++++++++++++++++++++++++-------- 2 files changed, 72 insertions(+), 21 deletions(-) diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index aedd8c2ce..b957c29fb 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -11,7 +11,7 @@ def _import_file(filepath): abspath = os.path.abspath(filepath) dirname, file = os.path.split(abspath) fname, fext = os.path.splitext(file) - if fext != '.py': + if fext not in ('.py', '.pyw'): raise ValueError(f"Not a Python source file: {abspath}") if dirname: sys.path = [dirname] + sys.path diff --git a/tests/test_commands.py b/tests/test_commands.py index 2899e5f24..3e54a0948 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -496,6 +496,8 @@ class MiscCommandsTest(CommandTest): class RunSpiderCommandTest(CommandTest): + spider_filename = 'myspider.py' + debug_log_spider = """ import scrapy @@ -507,11 +509,23 @@ class MySpider(scrapy.Spider): return [] """ + badspider = """ +import scrapy + +class BadSpider(scrapy.Spider): + name = "bad" + def start_requests(self): + raise Exception("oops!") + """ + @contextmanager - def _create_file(self, content, name): + def _create_file(self, content, name=None): tmpdir = self.mktemp() os.mkdir(tmpdir) - fname = abspath(join(tmpdir, name)) + if name: + fname = abspath(join(tmpdir, name)) + else: + fname = abspath(join(tmpdir, self.spider_filename)) with open(fname, 'w') as f: f.write(content) try: @@ -519,12 +533,12 @@ class MySpider(scrapy.Spider): finally: rmtree(tmpdir) - def runspider(self, code, name='myspider.py', args=()): + def runspider(self, code, name=None, args=()): with self._create_file(code, name) as fname: return self.proc('runspider', fname, *args) - def get_log(self, code, name='myspider.py', args=()): - p, stdout, stderr = self.runspider(code, name=name, args=args) + def get_log(self, code, name=None, args=()): + p, stdout, stderr = self.runspider(code, name, args=args) return stderr def test_runspider(self): @@ -556,7 +570,7 @@ class MySpider(scrapy.Spider): # which is intended, # but this should not be because of DNS lookup error # assumption: localhost will resolve in all cases (true?) - log = self.get_log(""" + dnscache_spider = """ import scrapy class MySpider(scrapy.Spider): @@ -565,23 +579,20 @@ class MySpider(scrapy.Spider): def parse(self, response): return {'test': 'value'} -""", - args=('-s', 'DNSCACHE_ENABLED=False')) - print(log) +""" + log = self.get_log(dnscache_spider, args=('-s', 'DNSCACHE_ENABLED=False')) self.assertNotIn("DNSLookupError", log) self.assertIn("INFO: Spider opened", log) def test_runspider_log_short_names(self): log1 = self.get_log(self.debug_log_spider, args=('-s', 'LOG_SHORT_NAMES=1')) - print(log1) self.assertIn("[myspider] DEBUG: It Works!", log1) self.assertIn("[scrapy]", log1) self.assertNotIn("[scrapy.core.engine]", log1) log2 = self.get_log(self.debug_log_spider, args=('-s', 'LOG_SHORT_NAMES=0')) - print(log2) self.assertIn("[myspider] DEBUG: It Works!", log2) self.assertNotIn("[scrapy]", log2) self.assertIn("[scrapy.core.engine]", log2) @@ -599,15 +610,7 @@ class MySpider(scrapy.Spider): self.assertIn('Unable to load', log) def test_start_requests_errors(self): - log = self.get_log(""" -import scrapy - -class BadSpider(scrapy.Spider): - name = "bad" - def start_requests(self): - raise Exception("oops!") - """, name="badspider.py") - print(log) + log = self.get_log(self.badspider, name='badspider.py') self.assertIn("start_requests", log) self.assertIn("badspider.py", log) @@ -696,6 +699,54 @@ class MySpider(scrapy.Spider): self.assertIn("error: Please use only one of -o/--output and -O/--overwrite-output", log) +class WindowsRunSpiderCommandTest(RunSpiderCommandTest): + + spider_filename = 'myspider.pyw' + + def setUp(self): + super(WindowsRunSpiderCommandTest, self).setUp() + + def test_start_requests_errors(self): + log = self.get_log(self.badspider, name='badspider.pyw') + self.assertIn("start_requests", log) + self.assertIn("badspider.pyw", log) + + @skipIf(platform.system() != 'Windows', "Windows required for .pyw files") + def test_run_good_spider(self): + super().test_run_good_spider() + + @skipIf(platform.system() != 'Windows', "Windows required for .pyw files") + def test_runspider(self): + super().test_runspider() + + @skipIf(platform.system() != 'Windows', "Windows required for .pyw files") + def test_runspider_dnscache_disabled(self): + super().test_runspider_dnscache_disabled() + + @skipIf(platform.system() != 'Windows', "Windows required for .pyw files") + def test_runspider_log_level(self): + super().test_runspider_log_level() + + @skipIf(platform.system() != 'Windows', "Windows required for .pyw files") + def test_runspider_log_short_names(self): + super().test_runspider_log_short_names() + + @skipIf(platform.system() != 'Windows', "Windows required for .pyw files") + def test_runspider_no_spider_found(self): + super().test_runspider_no_spider_found() + + @skipIf(platform.system() != 'Windows', "Windows required for .pyw files") + def test_output(self): + super().test_output() + + @skipIf(platform.system() != 'Windows', "Windows required for .pyw files") + def test_overwrite_output(self): + super().test_overwrite_output() + + def test_runspider_unable_to_load(self): + raise unittest.SkipTest("Already Tested in 'RunSpiderCommandTest' ") + + class BenchCommandTest(CommandTest): def test_run(self): From 797a6690c07ae90f7a2d703832e2fe44466d656f Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <1731933+elacuesta@users.noreply.github.com> Date: Thu, 1 Oct 2020 23:11:11 -0300 Subject: [PATCH 32/34] Tests: use classes instead of paths in settings (#4817) --- tests/test_dupefilters.py | 10 +-- tests/test_logformatter.py | 4 +- tests/test_pipelines.py | 2 +- tests/test_request_attribute_binding.py | 14 ++-- tests/test_request_cb_kwargs.py | 4 +- tests/test_spidermiddleware_output_chain.py | 92 ++++++++++----------- 6 files changed, 63 insertions(+), 63 deletions(-) diff --git a/tests/test_dupefilters.py b/tests/test_dupefilters.py index 95a4fca0d..680bb6dc8 100644 --- a/tests/test_dupefilters.py +++ b/tests/test_dupefilters.py @@ -43,7 +43,7 @@ class RFPDupeFilterTest(unittest.TestCase): def test_df_from_crawler_scheduler(self): settings = {'DUPEFILTER_DEBUG': True, - 'DUPEFILTER_CLASS': __name__ + '.FromCrawlerRFPDupeFilter'} + 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter} crawler = get_crawler(settings_dict=settings) scheduler = Scheduler.from_crawler(crawler) self.assertTrue(scheduler.df.debug) @@ -51,14 +51,14 @@ class RFPDupeFilterTest(unittest.TestCase): def test_df_from_settings_scheduler(self): settings = {'DUPEFILTER_DEBUG': True, - 'DUPEFILTER_CLASS': __name__ + '.FromSettingsRFPDupeFilter'} + 'DUPEFILTER_CLASS': FromSettingsRFPDupeFilter} crawler = get_crawler(settings_dict=settings) scheduler = Scheduler.from_crawler(crawler) self.assertTrue(scheduler.df.debug) self.assertEqual(scheduler.df.method, 'from_settings') def test_df_direct_scheduler(self): - settings = {'DUPEFILTER_CLASS': __name__ + '.DirectDupeFilter'} + settings = {'DUPEFILTER_CLASS': DirectDupeFilter} crawler = get_crawler(settings_dict=settings) scheduler = Scheduler.from_crawler(crawler) self.assertEqual(scheduler.df.method, 'n/a') @@ -162,7 +162,7 @@ class RFPDupeFilterTest(unittest.TestCase): def test_log(self): with LogCapture() as log: settings = {'DUPEFILTER_DEBUG': False, - 'DUPEFILTER_CLASS': __name__ + '.FromCrawlerRFPDupeFilter'} + 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter} crawler = get_crawler(SimpleSpider, settings_dict=settings) scheduler = Scheduler.from_crawler(crawler) spider = SimpleSpider.from_crawler(crawler) @@ -191,7 +191,7 @@ class RFPDupeFilterTest(unittest.TestCase): def test_log_debug(self): with LogCapture() as log: settings = {'DUPEFILTER_DEBUG': True, - 'DUPEFILTER_CLASS': __name__ + '.FromCrawlerRFPDupeFilter'} + 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter} crawler = get_crawler(SimpleSpider, settings_dict=settings) scheduler = Scheduler.from_crawler(crawler) spider = SimpleSpider.from_crawler(crawler) diff --git a/tests/test_logformatter.py b/tests/test_logformatter.py index dc5be398f..6381f895b 100644 --- a/tests/test_logformatter.py +++ b/tests/test_logformatter.py @@ -193,7 +193,7 @@ class ShowOrSkipMessagesTestCase(TwistedTestCase): self.base_settings = { 'LOG_LEVEL': 'DEBUG', 'ITEM_PIPELINES': { - __name__ + '.DropSomeItemsPipeline': 300, + DropSomeItemsPipeline: 300, }, } @@ -212,7 +212,7 @@ class ShowOrSkipMessagesTestCase(TwistedTestCase): @defer.inlineCallbacks def test_skip_messages(self): settings = self.base_settings.copy() - settings['LOG_FORMATTER'] = __name__ + '.SkipMessagesLogFormatter' + settings['LOG_FORMATTER'] = SkipMessagesLogFormatter crawler = CrawlerRunner(settings).create_crawler(ItemSpider) with LogCapture() as lc: yield crawler.crawl(mockserver=self.mockserver) diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index c72f1a338..ff3af9a74 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -68,7 +68,7 @@ class PipelineTestCase(unittest.TestCase): def _create_crawler(self, pipeline_class): settings = { - 'ITEM_PIPELINES': {__name__ + '.' + pipeline_class.__name__: 1}, + 'ITEM_PIPELINES': {pipeline_class: 1}, } crawler = get_crawler(ItemSpider, settings) crawler.signals.connect(self._on_item_scraped, signals.item_scraped) diff --git a/tests/test_request_attribute_binding.py b/tests/test_request_attribute_binding.py index 907117468..00c532c41 100644 --- a/tests/test_request_attribute_binding.py +++ b/tests/test_request_attribute_binding.py @@ -92,7 +92,7 @@ class CrawlTestCase(TestCase): url = self.mockserver.url("/status?n=200") runner = CrawlerRunner(settings={ "DOWNLOADER_MIDDLEWARES": { - __name__ + ".RaiseExceptionRequestMiddleware": 590, + RaiseExceptionRequestMiddleware: 590, }, }) crawler = runner.create_crawler(SingleRequestSpider) @@ -119,7 +119,7 @@ class CrawlTestCase(TestCase): url = self.mockserver.url("/status?n=200") runner = CrawlerRunner(settings={ "DOWNLOADER_MIDDLEWARES": { - __name__ + ".ProcessResponseMiddleware": 595, + ProcessResponseMiddleware: 595, } }) crawler = runner.create_crawler(SingleRequestSpider) @@ -149,8 +149,8 @@ class CrawlTestCase(TestCase): url = self.mockserver.url("/status?n=200") runner = CrawlerRunner(settings={ "DOWNLOADER_MIDDLEWARES": { - __name__ + ".RaiseExceptionRequestMiddleware": 590, - __name__ + ".CatchExceptionOverrideRequestMiddleware": 595, + RaiseExceptionRequestMiddleware: 590, + CatchExceptionOverrideRequestMiddleware: 595, }, }) crawler = runner.create_crawler(SingleRequestSpider) @@ -170,8 +170,8 @@ class CrawlTestCase(TestCase): url = self.mockserver.url("/status?n=200") runner = CrawlerRunner(settings={ "DOWNLOADER_MIDDLEWARES": { - __name__ + ".RaiseExceptionRequestMiddleware": 590, - __name__ + ".CatchExceptionDoNotOverrideRequestMiddleware": 595, + RaiseExceptionRequestMiddleware: 590, + CatchExceptionDoNotOverrideRequestMiddleware: 595, }, }) crawler = runner.create_crawler(SingleRequestSpider) @@ -188,7 +188,7 @@ class CrawlTestCase(TestCase): """ runner = CrawlerRunner(settings={ "DOWNLOADER_MIDDLEWARES": { - __name__ + ".AlternativeCallbacksMiddleware": 595, + AlternativeCallbacksMiddleware: 595, } }) crawler = runner.create_crawler(AlternativeCallbacksSpider) diff --git a/tests/test_request_cb_kwargs.py b/tests/test_request_cb_kwargs.py index bd49179aa..145a4e9b2 100644 --- a/tests/test_request_cb_kwargs.py +++ b/tests/test_request_cb_kwargs.py @@ -50,10 +50,10 @@ class KeywordArgumentsSpider(MockServerSpider): name = 'kwargs' custom_settings = { 'DOWNLOADER_MIDDLEWARES': { - __name__ + '.InjectArgumentsDownloaderMiddleware': 750, + InjectArgumentsDownloaderMiddleware: 750, }, 'SPIDER_MIDDLEWARES': { - __name__ + '.InjectArgumentsSpiderMiddleware': 750, + InjectArgumentsSpiderMiddleware: 750, }, } diff --git a/tests/test_spidermiddleware_output_chain.py b/tests/test_spidermiddleware_output_chain.py index 2f454addc..029bf8bd6 100644 --- a/tests/test_spidermiddleware_output_chain.py +++ b/tests/test_spidermiddleware_output_chain.py @@ -16,11 +16,20 @@ class LogExceptionMiddleware: # ================================================================================ # (0) recover from an exception on a spider callback +class RecoveryMiddleware: + def process_spider_exception(self, response, exception, spider): + spider.logger.info('Middleware: %s exception caught', exception.__class__.__name__) + return [ + {'from': 'process_spider_exception'}, + Request(response.url, meta={'dont_fail': True}, dont_filter=True), + ] + + class RecoverySpider(Spider): name = 'RecoverySpider' custom_settings = { 'SPIDER_MIDDLEWARES': { - __name__ + '.RecoveryMiddleware': 10, + RecoveryMiddleware: 10, }, } @@ -34,15 +43,6 @@ class RecoverySpider(Spider): raise TabError() -class RecoveryMiddleware: - def process_spider_exception(self, response, exception, spider): - spider.logger.info('Middleware: %s exception caught', exception.__class__.__name__) - return [ - {'from': 'process_spider_exception'}, - Request(response.url, meta={'dont_fail': True}, dont_filter=True), - ] - - # ================================================================================ # (1) exceptions from a spider middleware's process_spider_input method class FailProcessSpiderInputMiddleware: @@ -56,9 +56,8 @@ class ProcessSpiderInputSpiderWithoutErrback(Spider): custom_settings = { 'SPIDER_MIDDLEWARES': { # spider - __name__ + '.LogExceptionMiddleware': 10, - __name__ + '.FailProcessSpiderInputMiddleware': 8, - __name__ + '.LogExceptionMiddleware': 6, + FailProcessSpiderInputMiddleware: 8, + LogExceptionMiddleware: 6, # engine } } @@ -87,7 +86,7 @@ class GeneratorCallbackSpider(Spider): name = 'GeneratorCallbackSpider' custom_settings = { 'SPIDER_MIDDLEWARES': { - __name__ + '.LogExceptionMiddleware': 10, + LogExceptionMiddleware: 10, }, } @@ -106,7 +105,7 @@ class GeneratorCallbackSpiderMiddlewareRightAfterSpider(GeneratorCallbackSpider) name = 'GeneratorCallbackSpiderMiddlewareRightAfterSpider' custom_settings = { 'SPIDER_MIDDLEWARES': { - __name__ + '.LogExceptionMiddleware': 100000, + LogExceptionMiddleware: 100000, }, } @@ -117,7 +116,7 @@ class NotGeneratorCallbackSpider(Spider): name = 'NotGeneratorCallbackSpider' custom_settings = { 'SPIDER_MIDDLEWARES': { - __name__ + '.LogExceptionMiddleware': 10, + LogExceptionMiddleware: 10, }, } @@ -134,32 +133,13 @@ class NotGeneratorCallbackSpiderMiddlewareRightAfterSpider(NotGeneratorCallbackS name = 'NotGeneratorCallbackSpiderMiddlewareRightAfterSpider' custom_settings = { 'SPIDER_MIDDLEWARES': { - __name__ + '.LogExceptionMiddleware': 100000, + LogExceptionMiddleware: 100000, }, } # ================================================================================ # (4) exceptions from a middleware process_spider_output method (generator) -class GeneratorOutputChainSpider(Spider): - name = 'GeneratorOutputChainSpider' - custom_settings = { - 'SPIDER_MIDDLEWARES': { - __name__ + '.GeneratorFailMiddleware': 10, - __name__ + '.GeneratorDoNothingAfterFailureMiddleware': 8, - __name__ + '.GeneratorRecoverMiddleware': 5, - __name__ + '.GeneratorDoNothingAfterRecoveryMiddleware': 3, - }, - } - - def start_requests(self): - yield Request(self.mockserver.url('/status?n=200')) - - def parse(self, response): - yield {'processed': ['parse-first-item']} - yield {'processed': ['parse-second-item']} - - class _GeneratorDoNothingMiddleware: def process_spider_output(self, response, result, spider): for r in result: @@ -205,26 +185,28 @@ class GeneratorDoNothingAfterRecoveryMiddleware(_GeneratorDoNothingMiddleware): pass -# ================================================================================ -# (5) exceptions from a middleware process_spider_output method (not generator) -class NotGeneratorOutputChainSpider(Spider): - name = 'NotGeneratorOutputChainSpider' +class GeneratorOutputChainSpider(Spider): + name = 'GeneratorOutputChainSpider' custom_settings = { 'SPIDER_MIDDLEWARES': { - __name__ + '.NotGeneratorFailMiddleware': 10, - __name__ + '.NotGeneratorDoNothingAfterFailureMiddleware': 8, - __name__ + '.NotGeneratorRecoverMiddleware': 5, - __name__ + '.NotGeneratorDoNothingAfterRecoveryMiddleware': 3, + GeneratorFailMiddleware: 10, + GeneratorDoNothingAfterFailureMiddleware: 8, + GeneratorRecoverMiddleware: 5, + GeneratorDoNothingAfterRecoveryMiddleware: 3, }, } def start_requests(self): - return [Request(self.mockserver.url('/status?n=200'))] + yield Request(self.mockserver.url('/status?n=200')) def parse(self, response): - return [{'processed': ['parse-first-item']}, {'processed': ['parse-second-item']}] + yield {'processed': ['parse-first-item']} + yield {'processed': ['parse-second-item']} +# ================================================================================ +# (5) exceptions from a middleware process_spider_output method (not generator) + class _NotGeneratorDoNothingMiddleware: def process_spider_output(self, response, result, spider): out = [] @@ -276,6 +258,24 @@ class NotGeneratorDoNothingAfterRecoveryMiddleware(_NotGeneratorDoNothingMiddlew pass +class NotGeneratorOutputChainSpider(Spider): + name = 'NotGeneratorOutputChainSpider' + custom_settings = { + 'SPIDER_MIDDLEWARES': { + NotGeneratorFailMiddleware: 10, + NotGeneratorDoNothingAfterFailureMiddleware: 8, + NotGeneratorRecoverMiddleware: 5, + NotGeneratorDoNothingAfterRecoveryMiddleware: 3, + }, + } + + def start_requests(self): + return [Request(self.mockserver.url('/status?n=200'))] + + def parse(self, response): + return [{'processed': ['parse-first-item']}, {'processed': ['parse-second-item']}] + + # ================================================================================ class TestSpiderMiddleware(TestCase): @classmethod From 004b40a7193a7c0c5138abe764cad6d1d9a4fc57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Sat, 3 Oct 2020 00:53:55 +0200 Subject: [PATCH 33/34] =?UTF-8?q?as=20soon=20as=20=E2=86=92=20as=20long=20?= =?UTF-8?q?as=20(#4825)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 675f55c38..4d2580a6c 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -140,7 +140,7 @@ original pull request author hasn't had time to address them. In this case consider picking up this pull request: open a new pull request with all commits from the original pull request, as well as additional changes to address the raised issues. Doing so helps a lot; it is -not considered rude as soon as the original author is acknowledged by keeping +not considered rude as long as the original author is acknowledged by keeping his/her commits. You can pull an existing pull request to a local branch From 9b1f86b613d2039b0a66ba2b527a7e9bffadaf3a Mon Sep 17 00:00:00 2001 From: Andrey Rahmatullin Date: Tue, 6 Oct 2020 18:50:55 +0500 Subject: [PATCH 34/34] Use f-strings --- scrapy/utils/iterators.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index e140e3f6f..3b504e56a 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -35,7 +35,7 @@ def xmliter(obj, nodename): namespaces = {} if header_end: for tagname in reversed(re.findall(END_TAG_RE, header_end)): - tag = re.search(r'<\s*%s.*?xmlns[:=][^>]*>' % tagname, text[:header_end_idx[1]], re.S) + tag = re.search(fr'<\s*{tagname}.*?xmlns[:=][^>]*>', text[:header_end_idx[1]], re.S) if tag: namespaces.update(reversed(x) for x in re.findall(NAMESPACE_RE, tag.group())) @@ -45,7 +45,7 @@ def xmliter(obj, nodename): document_header + match.group().replace( nodename, - '%s %s' % (nodename, ' '.join(namespaces.values())), + f'{nodename} {" ".join(namespaces.values())}', 1 ) + header_end @@ -56,7 +56,7 @@ def xmliter(obj, nodename): def xmliter_lxml(obj, nodename, namespace=None, prefix='x'): from lxml import etree reader = _StreamReader(obj) - tag = f'{{{namespace}}}{nodename}'if namespace else nodename + tag = f'{{{namespace}}}{nodename}' if namespace else nodename iterable = etree.iterparse(reader, tag=tag, encoding=reader.encoding) selxpath = '//' + (f'{prefix}:{nodename}' if namespace else nodename) for _, node in iterable: