Drop versions earlier than Python 3.7 and update requirements #159
Conversation
.envrc
Outdated
| @@ -0,0 +1 @@ | |||
| export SH_APIKEY='' | |||
There was a problem hiding this comment.
Why is this file Git-tracked?
There was a problem hiding this comment.
It's to make it evident that some tests will fail if SH_APIKEY is defined in the environment, as is the case for most Zyte developers.
There was a problem hiding this comment.
We could instead force this in tox, since tox is the standard way to run tests in this project.
This file would only be useful for developers using direnv, which may be or become true for most Zytans, but probably won’t for the wider open source community.
There was a problem hiding this comment.
I agree. I'll add the envvar to tox.ini and document that tox (and not pytest) should be use to run the tests.
setup.py
Outdated
| @@ -26,20 +26,20 @@ | |||
| platforms=['Any'], | |||
| packages=['scrapinghub', 'scrapinghub.client', 'scrapinghub.hubstorage'], | |||
| package_data={'scrapinghub': ['VERSION']}, | |||
| install_requires=['requests>=1.0', 'retrying>=1.3.3', 'six>=1.10.0'], | |||
| extras_require={'msgpack': mpack_required}, | |||
| install_requires= mpack_required + [ | |||
There was a problem hiding this comment.
Why is msgpack no longer optional?
There was a problem hiding this comment.
shub doesn't work well without msgpack. Numerous requests for help on the #devel channel end up with "install msgpack, and get rid of mspack-python).
There was a problem hiding this comment.
I have just found out that removing the entry from extras_require is backward incompatible, so I suggest we keep it with an empty list of requirements, as suggested in the setuptools documentation:
Best practice: if a project ends up not needing any other packages to support a feature, it should keep an empty requirements list for that feature in its extras_require argument, so that packages depending on that feature don’t break (due to an invalid feature name).
There was a problem hiding this comment.
(I would usually provide a suggestion, but I needed a fork I could clone for immediate usage)
| @@ -177,6 +177,7 @@ def test_summary_countstart(hsproject): | |||
| [o['key'] for o in s2['summary'][-6:-3]]) | |||
|
|
|||
|
|
|||
| @pytest.mark.skip(reason='Servers not running') | |||
There was a problem hiding this comment.
Can you elaborate? What do we need to do to get these tests working?
There was a problem hiding this comment.
To run those tests we need a devbox.
Kumo has one or more of those, so they can help with the testing (access to an internal dev machine?) when the time is right.
There was a problem hiding this comment.
Note that having tests that require a live box means that we couldn't run the tests in Github actions.
|
|
|
we need to move this forward, tests are failing becuase of cassette replay problems. I also don't think we need to bother about upgrading all libraries and dropping python 2 support NOW. We need to have this working with python 3.10 asap and worry about other things later. I'll try to make other smaller PR with only essential things (edited) |
|
@pawelmhm, the upgrade on this pull request is all we need. I know because I've tested it enough. Everything else can wait. |
|
Python 3.10 support added in #166 |
library upgrades
This update nominally drops support for Python 2.7, 3.5, and 3.6, and tests support for 3.10 to avoid libraries being pinned to very old versions, many of them with bugs or with security issues.
It's "nominally" because the code hasn't been changed except for deprecations enforced in Python 3.9 or 3.10.
disabled tests
Tests that required running test servers are disabled:
The tests can be re-enabled by someone with access to test servers.