Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Python 3.8 official support #4092

Merged
merged 21 commits into from Oct 29, 2019
Merged

Add Python 3.8 official support #4092

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3d4317b
[tox.ini] Added python 3.8 fields
further-reading Oct 21, 2019
4e939ca
[setup.py] Added python 3.8 fields
further-reading Oct 21, 2019
c12a075
[.travis.yml] Added python 3.8 fields
further-reading Oct 21, 2019
85ac5c5
Update .travis.yml
further-reading Oct 21, 2019
179dc91
Update setup.py
further-reading Oct 27, 2019
4068797
Update test_downloadermiddleware_httpcache.py
further-reading Oct 27, 2019
deacd34
[test_downloadermiddleware_httpcache] Attempting to add xfail for lev…
further-reading Oct 27, 2019
11942c4
[test_downloadermiddleware_httpcache] Trying hack to handle systemerr…
further-reading Oct 27, 2019
b3df0a8
[test_downloadermiddleware_httpcache] Adding xfails to impacted tests…
further-reading Oct 27, 2019
70b2854
[test_downloadermiddleware_httpcache] Making xfails more informative
further-reading Oct 27, 2019
20ea912
[test_downloadermiddleware_httpcache] Making xfails more informative
further-reading Oct 27, 2019
b5a0026
Update .travis.yml
further-reading Oct 28, 2019
3d0df41
Mark the LevelDB storage backend as deprecated
Gallaecio Oct 28, 2019
3af2abb
Merge pull request #1 from Gallaecio/py38
further-reading Oct 28, 2019
16bb3ac
[test_downloadermiddleware_httpcache] Using skipif approach
further-reading Oct 28, 2019
9b47dc6
[travis, setup] Adding official python 3.8 support
further-reading Oct 28, 2019
4432136
[test_downloadermiddleware_httpcache] Fixing pytest skip behaviour
further-reading Oct 28, 2019
c51fb95
[test_downloadermiddleware_httpcache] Fixing pytest skip behaviour
further-reading Oct 28, 2019
7490903
[tox.ini] Removing obsolete py37 extra deps enviornment
further-reading Oct 28, 2019
b73d217
[test_downloadermiddleware_httpcache.py] Fixing pytest mark behaviour
further-reading Oct 28, 2019
93e3dc1
[test_downloadermiddleware_httpcache.py] Cleaning text
further-reading Oct 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions .travis.yml
View file
Open in desktop
Expand Up @@ -25,8 +25,10 @@ matrix:
python: 3.6
- env: TOXENV=py37
python: 3.7
- env: TOXENV=py37-extra-deps
python: 3.7
- env: TOXENV=py38
python: 3.8
- env: TOXENV=py38-extra-deps
further-reading marked this conversation as resolved.
Show resolved Hide resolved
python: 3.8
- env: TOXENV=docs
python: 3.6
install:
Expand Down
22 changes: 0 additions & 22 deletions docs/topics/downloader-middleware.rst
View file
Open in desktop
Expand Up @@ -348,7 +348,6 @@ HttpCacheMiddleware

* :ref:`httpcache-storage-fs`
* :ref:`httpcache-storage-dbm`
* :ref:`httpcache-storage-leveldb`

You can change the HTTP cache storage backend with the :setting:`HTTPCACHE_STORAGE`
setting. Or you can also :ref:`implement your own storage backend. <httpcache-storage-custom>`
Expand Down Expand Up @@ -478,27 +477,6 @@ DBM storage backend
By default, it uses the anydbm_ module, but you can change it with the
:setting:`HTTPCACHE_DBM_MODULE` setting.

.. _httpcache-storage-leveldb:

LevelDB storage backend
~~~~~~~~~~~~~~~~~~~~~~~

.. class:: LeveldbCacheStorage

.. versionadded:: 0.23

A LevelDB_ storage backend is also available for the HTTP cache middleware.

This backend is not recommended for development because only one process
can access LevelDB databases at the same time, so you can't run a crawl and
open the scrapy shell in parallel for the same spider.

In order to use this storage backend, install the `LevelDB python
bindings`_ (e.g. ``pip install leveldb``).

.. _LevelDB: https://github.com/google/leveldb
.. _leveldb python bindings: https://pypi.python.org/pypi/leveldb

.. _httpcache-storage-custom:

Writing your own storage backend
Expand Down
17 changes: 12 additions & 5 deletions scrapy/extensions/httpcache.py
View file
Open in desktop
@@ -1,19 +1,24 @@
from __future__ import print_function
import os

import gzip
import logging
from six.moves import cPickle as pickle
import os
from email.utils import mktime_tz, parsedate_tz
from importlib import import_module
from time import time
from warnings import warn
from weakref import WeakKeyDictionary
from email.utils import mktime_tz, parsedate_tz

from six.moves import cPickle as pickle
from w3lib.http import headers_raw_to_dict, headers_dict_to_raw

from scrapy.exceptions import ScrapyDeprecationWarning
from scrapy.http import Headers, Response
from scrapy.responsetypes import responsetypes
from scrapy.utils.request import request_fingerprint
from scrapy.utils.project import data_path
from scrapy.utils.httpobj import urlparse_cached
from scrapy.utils.project import data_path
from scrapy.utils.python import to_bytes, to_unicode, garbage_collect
from scrapy.utils.request import request_fingerprint


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -345,6 +350,8 @@ def _read_meta(self, spider, request):
class LeveldbCacheStorage(object):

def __init__(self, settings):
warn("The LevelDB storage backend is deprecated.",
ScrapyDeprecationWarning, stacklevel=2)
import leveldb
self._leveldb = leveldb
self.cachedir = data_path(settings['HTTPCACHE_DIR'], createdir=True)
Expand Down
1 change: 1 addition & 0 deletions setup.py
View file
Open in desktop
Expand Up @@ -56,6 +56,7 @@ def has_environment_marker_platform_impl_support():
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: Implementation :: CPython',
'Programming Language :: Python :: Implementation :: PyPy',
'Topic :: Internet :: WWW/HTTP',
Expand Down
7 changes: 6 additions & 1 deletion tests/test_downloadermiddleware_httpcache.py
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@
import email.utils
from contextlib import contextmanager
import pytest
import sys
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💄 This does not seem to be used below.


from scrapy.http import Response, HtmlResponse, Request
from scrapy.spiders import Spider
Expand Down Expand Up @@ -156,7 +157,11 @@ def _get_settings(self, **new_settings):

class LeveldbStorageTest(DefaultStorageTest):

pytest.importorskip('leveldb')
try:
pytest.importorskip('leveldb')
except SystemError:
# Happens in python 3.8
pytestmark = pytest.skip("'SystemError: bad call flags' occurs on Python 3.8")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the assignment needed?

Suggested change
pytestmark = pytest.skip("'SystemError: bad call flags' occurs on Python 3.8")
pytest.skip("'SystemError: bad call flags' occurs on Python 3.8")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - travis CI called it out specifically in the last build https://travis-ci.org/scrapy/scrapy/jobs/603880139?utm_medium=notification&utm_source=github_status

Using pytest.skip outside of a test is not allowed. To decorate a test function, use the @pytest.mark.skip or @pytest.mark.skipif decorators instead, and to skip a module use `pytestmark = pytest.mark.{skip,skipif}.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting complex.

I think you may be able to move the try-except to something like https://docs.pytest.org/en/latest/xunit_setup.html#method-and-function-level-setup-teardown, but feel free to go back to your initial approach of decorators based on the running Python version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how using a setup method is gonna make it less complex? You mean making something like or is there another approach I'm missing?

class LeveldbStorageTest(DefaultStorageTest):
    storage_class = 'scrapy.extensions.httpcache.LeveldbCacheStorage'

    @classmethod
    def setup_class(cls, *args, **kwargs):
        test_class = cls(*args, **kwargs)
        try:
            test_class.leveldb = pytest.importorskip('leveldb')
        except SystemError:
            test_class.pytestmark = pytest.mark.skip("'SystemError: bad call flags' occurs on Python >= 3.8")

        return test_class

Right now I have it refactored to a single check localized to the specific subclass where the problem arises, with clear commenting/logging when it triggers. Reverting back to the multiple decorators or incorporating a setup method seems like it'd be adding complexity as I am adding adding checks to multiple parts of the code or I am having to incorporate additional syntax to mimic the current behavior.

storage_class = 'scrapy.extensions.httpcache.LeveldbCacheStorage'


Expand Down
11 changes: 11 additions & 0 deletions tox.ini
View file
Open in desktop
Expand Up @@ -92,6 +92,10 @@ deps = {[testenv:py35]deps}
basepython = python3.7
deps = {[testenv:py35]deps}

[testenv:py38]
basepython = python3.8
deps = {[testenv:py35]deps}

[testenv:pypy3]
basepython = pypy3
deps = {[testenv:py35]deps}
Expand Down Expand Up @@ -128,6 +132,13 @@ deps =
reppy
robotexclusionrulesparser
further-reading marked this conversation as resolved.
Show resolved Hide resolved

[testenv:py38-extra-deps]
basepython = python3.8
deps =
{[testenv:py35]deps}
reppy
robotexclusionrulesparser

[testenv:py27-extra-deps]
basepython = python2.7
deps =
Expand Down