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.

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

Consistency between setup.py and requirements.txt #724

Open
hoylen opened this issue Apr 3, 2020 · 1 comment
Open

Consistency between setup.py and requirements.txt #724

hoylen opened this issue Apr 3, 2020 · 1 comment

Comments

@hoylen
Copy link
Contributor

hoylen commented Apr 3, 2020

Describe the bug

The versions of package dependencies in setup.py and requirements.txt are inconsistent.

For the extra_requires, setup.py references these packages:

oauthlib/setup.py

Lines 21 to 23 in d4716eb

rsa_require = ['cryptography']
signedtoken_require = ['cryptography', 'pyjwt>=1.0.0']
signals_require = ['blinker']

The requirements.txt contains:

pyjwt==1.6.0
blinker==1.4
cryptography>=1.4.0

And there is also a requirements-test.txt that references packages not mentioned in setup.py:

-r requirements.txt
pytest>=4.0
pytest-cov>=2.6

It all seems a bit inconsistent: some packages don't have version numbers, different version numbers for the same package, and there is an odd mix of ">=" and "==" being used.

So it needs to be tidied up and documented (so users and developers know the convention being used and how to install it correctly).

Expected behavior

Here is a strawman proposal:

  • Remove the signedtoken extras and only have one rsa extras. The name is more precise and easier to remember (are there such a thing as non-RSA signed tokens?)
  • That rsa extras should only require pyjwt>=....
  • It does not require to include cryptography, because cryptography is a dependency of pyjwt. (There is an existing extras that only depends on cryptography, but it appears having cryptography without pyjwt doesn't do anything useful.)
  • I don't know what blinker does, so it needs to be documented so people know whether they need to install it or not. In setup.py, it will be referenced as blinker>=....
  • Add a test extras to setup.py, if that is needed.
  • Then have one requirements-something.txt file corresponding to each extras (i.e. a requirements-rsa.txt, requirements-signal.txt and requirements-test.txt). The requirements.txt should correspond to the base install_requires in setup.py, but since setup.py doesn't have one there won't be a requirements.txt file.
  • Each of the requirements-*.txt file should be produced by running pip freeze to dump a full list of dependencies and their dependencies. They will all only contain "==" version references.
    • So if someone runs "pip install -f ..." to install one of the requirements files, they will reproduce the working environment exactly: installing the exact version of every package that oauthlib was tested with.
    • But if they install via the setup.py file (e.g. "pip install oauthlib[rsa]") it may install newer version of various packages.

The above is based on the convention described by setup.py vs requirements.txt. It seems the convention is designed for libraries to be installed via setup.py (which only contains ">=" versions), so they are flexible and can use any version of a package (as long as it is compatible with version dependencies in other packages they are installing). But that risks installing a version that breaks the library. Whereas the requirement*.txt files are intended (for a developer?) to recreate a working set of packages: installing the exact versions of every single package from a known/tested environment.

I'm not an expert in this area, so is there a better approach?

This issue expands on the suggestion in #721 to merge the signedtoken and rsa extras.

The strawman proposes a way to consistently handling mandatory vs optional dependencies, which raised #383. OAuthLib does not have any mandatory dependencies, so the strawman proposes not to have any requirements.txt file.

@JonathanHuot
Copy link
Member

I agree with a lot of what you described.

To add more, I am not sure the pip install oauthlib[xyz] variants are used by anyone ?

Also, if someone want to propose a PR to migrate to pyproject.toml, that's will also remove some crapyness of the setup-things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants