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

[packaging] move list of python project from requirements.txt into install_requires in setup.py #18

Closed
mbukatov opened this issue Jan 3, 2017 · 6 comments

Comments

@mbukatov
Copy link
Contributor

mbukatov commented Jan 3, 2017

This issue suggest to remove requirements.txt file and move items which this file contains into install_requires setuptools keyword in setup.py file.

Reasoning

Based on explanation of difference between install_requires keyword and requirements.txt file from upstream Python Packaging User Guide, I understand that:

  • python project should provide minimal abstract list of projects which are needed to run it correctly in install_requires
  • requirements.txt files are used to define the requirements for a complete python environment

Since this project is a single Tendrl component, which is expected to be installed with other Tendrl components together, using requirement file seems to be inappropriate. Listing dependencies of single reusable python library/tool/daemon, which is our case here, is not mentioned among 4 common uses of Requirements files as listed in description of the feature in packaging guide. Moreover, it creates additional issues:

  • using requirements.txt file makes us to install the project in non-production way, which means that we will find problems with listing requirements later than we should, see eg. issue [packaging] setup.py doesn't contain list of requirements #17 - this is very blatant issue, but was not noticed by any developer, unit tests or CI - since all of those are using installation method which doesn't resemble what an admin using this project would do
  • keeping requirements.txt file while specifying install_requires in setup.py requires to maintain function to read requirements.txt file in setup.py, which is unnecessary maintenance burden
  • feature set of requirements.txt is higher and includes features which should not be done in production setup
@r0h4n
Copy link
Contributor

r0h4n commented Jan 5, 2017

We are using "install_requires" in setup.py, Contents of the requirements.txt are automatically extracted and fed to "install_requires" [0], currently the performance_monitoring and alerting projects do not address that, @anmolbabu please refer to setup.py [0]

0 : https://github.com/Tendrl/node_agent/blob/master/setup.py#L25

@mbukatov
Copy link
Contributor Author

mbukatov commented Jan 5, 2017

@r0h4n my point here is that I would rather not use requirements.txt file at all, and use install_requires in setup.py only - does this make sense for you?

@mbukatov
Copy link
Contributor Author

mbukatov commented Jan 5, 2017

This topic was discussed on a meeting today, the summary follows:

  • the current dev work flow uses pip freeze to generate requirements.txt file on purpose
  • we agreed to keep requirements.txt files as primary and the only source of requirements (install_requires in setup.py is defined based on content of this file)
  • we agreed to drop explicit usage of pip install -r requirements.txt from CI configuration (eg. tox.ini), so that the connection between requirements.txt and install_requires is checked early

I'm going to propose a pull request witch tox.ini configuration change for the last point.

@mbukatov
Copy link
Contributor Author

mbukatov commented Jan 6, 2017

Based on agreement described in #18 (comment) (keep requirements.txt file), I'm closing this issue.

Btw: Issue with missing installation requirements in setup.py is tracked in #17

@mbukatov
Copy link
Contributor Author

mbukatov commented Jan 9, 2017

After checking how tox.ini and travis are wired up during work on #24, I feel that we should not maintain requirements via pip freeze generated requirements.txt file.

anmolbabu pushed a commit to anmolbabu/alerting that referenced this issue Mar 23, 2017
Option install_requires in setup.py contains names of all python modules
tednrl-alerting depends on, including tendrl-alerting.

For development and CI purposes, there are requirements.txt files as
well. The main difference between them is the github branch they use to
install other tendrl components from:

* requirements.develop.txt (uses develop branch)
* requirements.master.txt (uses master branch)

So in CI or development enviroment, one can install tendrl-alerting
into virtualenv like this::

    pip install -r requirements.master.txt

Based on description from:

* https://packaging.python.org/requirements/
* https://caremad.io/posts/2013/07/setup-vs-requirement/

tendrl-bug-id: Tendrl#17
tendrl-bug-id: Tendrl#18
tendrl-bug-id: Tendrl#37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants