devopshq / artifactory

dohq-artifactory: a Python client for Artifactory
https://devopshq.github.io/artifactory/
MIT License
269 stars 137 forks source link

`requests` dependency is unversioned, but at least 2.26.0 is not supported #414

Closed tk-woven closed 1 year ago

tk-woven commented 1 year ago

Issue

setup.py places no version requirements on requests, and requirements.dev.txt installs the latest version (2.28.0 at the time of this writing). Somewhere between 2.26.0 and 2.28.0, the fully-qualified requests.compat.JSONDecodeError pathname changed (or did not exist at all).

Resolution

Determine and place a minimum version restriction in setup.py or add support in exceptions.py for backward compatibility.

try:
    from requests.compat import JSONDecodeError
except ImportError:
    from json import JSONDecodeError
Stealthii commented 1 year ago

I made the changes within that module during #354. What version of Python are you using?

I see that in https://github.com/psf/requests/commit/8bce583 (which was first included in v2.28.0) changes to this module to drop support for Python 2.7 and 3.6 were made, however the logic to try importing from simplejson with a fallback to json is still included.

I've ran with the latest dohq-artifactory and requests packages against Python 3.8-3.10 and not encountered this issue.

Stealthii commented 1 year ago

Strike that, I see https://github.com/psf/requests/commit/db575ee made a change for v2.27.0 that introduces requests.exceptions.JSONDecodeError to decrease inconsistencies in the library.

We could either change the exception we handle and put a hard dependency on >2.27, or handle both, which would be my preferred option.

Raised #416 to address this issue.

tk-woven commented 1 year ago

Sorry for my delay, and thanks for addressing this! I looked over #/416. The underlying problem in this issue is that older versions of requests don't define requests.compat.JSONDecodeError. So unfortunately

except (requests.compat.JSONDecodeError, requests.exceptions.JSONDecodeError):

will still fail when using older versions of requests as the interpreter will not know what requests.compat.JSONDecodeError is.

To reproduce:

git clone git@github.com:devopshq/artifactory.git
python -m venv .venv
source .venv/bin/activate
python -m pip install -r requirements-dev.txt
python -m pip install --force-reinstall requests==2.26.0
<apply patch: https://github.com/devopshq/artifactory/pull/416/commits/b0b29182f0ccfd8a62d0925fa64a989264a7f8ef>
python -m pytest tests/unit/test_exceptions.py
AttributeError: module 'requests.compat' has no attribute 'JSONDecodeError'

If it's still useful to know the version of Python I'm using, it is 3.9 in this case.