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

Add requirements.txt for Python smoke tests. #289

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cjerdonek
Copy link

Hi!

I was reading the README for the smoketest directory and noticed that you can use a requirements.txt file. That's a best practice that simplifies test setup and also ensures that test environments are deterministic / identical across users (by pinning dependencies).

@nealmcb
Copy link
Contributor

nealmcb commented Aug 16, 2017

Wow - thanks, Chris! This is a model PR with just the sort of help folks should indeed be able to use!

And I just learned some nice things about Checking out pull requests locally - User Documentation, e.g. this syntax for doing a git fetch command by PR:

git fetch origin pull/289/head:smoketest-add-requirements-txt

Do you know if venv works in RHEL 7? I find following the instructions in your link to virtual environment doesn't work on Ubuntu Trusty:

$ python3 -m venv ColoradoRLA-venv                                                    
The virtual environment was not created successfully because ensurepip is not
available.  On Debian/Ubuntu systems, you need to install the python3-venv
package using the following command.

    apt-get install python3-venv

You may need to use sudo with that command.  After installing the python3-venv
package, recreate your virtual environment.

$ wajig install python3-venv
Reading package lists... Done
Building dependency tree       
Reading state information... Done
E: Unable to locate package python3-venv

@cjerdonek
Copy link
Author

Hi Neal! Yes, I know it works, but I don't have recent experience installing it on Ubuntu via python3-venv. In my own environments, I'm currently installing Python from source rather than using the system apt-get installed Python, so things are a bit different. I'm guessing your error is because you didn't run the install command using sudo, as the instructions say you might need (and it's consistent IIRC with "unable to locate" errors).

It's good for you to be nailing this down though and documenting it, because once you can create virtualenvs on your system, everything becomes "easy." Many Python folks view virtualenvs as essential for working day-to-day.

@nealmcb
Copy link
Contributor

nealmcb commented Aug 16, 2017

Indeed. I've used mkvirtualenv for years! Just trying to see how those exact instructions would work with venv.
wajig is an apt helper app that automatically does stuff like running sudo for you.

This works, it turns out: wajig install python3.4-venv
thanks to the instructions here:
https://bugs.launchpad.net/ubuntu/+source/python3.4/+bug/1532231

So the instructions on that page need some help on at least some environments.

@cjerdonek
Copy link
Author

Ah, cool, glad you figured it out so quickly! (Btw, I misunderstood the "unable to locate" error as being unable to locate the package after installation, but now I see it meant it couldn't locate it in the package management repository.)

# The sibling requirements.txt file contains the corresponding concrete /
# pinned dependencies. You can use pip-tools to automatically update
# requirements.txt from this file:
# https://github.com/jazzband/pip-tools
Copy link
Contributor

@nealmcb nealmcb Aug 16, 2017

Choose a reason for hiding this comment

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

Interesting. But I'm normally reluctant to introduce new tooling requirements. Can you contrast this with other options for maintaining requirements.txt?

Copy link
Author

Choose a reason for hiding this comment

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

pip-tools wouldn't be mandatory. It would only be useful to / potentially used by someone who wanted to update requirements.txt, e.g. to check for upgrades in package dependencies and/or when adding a new abstract dependency.

One possible "manual" approach to updating requirements.txt would be--

  1. Create a fresh virtualenv
  2. Manually invoke pip install ... with whatever abstract dependencies you want (e.g. requests, etc).
  3. Write the output of pip freeze to a file (e.g. pip freeze > requirements.txt.

(This is essentially what pip-tools does.)

One reason I mentioned pip-tools is that it lets you specify the abstract dependencies in a stand-alone requirements.in file, which is nice. I'm not sure other tools like pip recognize such a file format (though they might).

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I confirmed another option to updating requirements.txt is to run the following after creating a fresh virtualenv:

pip install -r requirements.in
pip freeze > requirements.txt

Copy link
Contributor

Choose a reason for hiding this comment

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

Super. Can you update the PR to do it that way?

Copy link
Contributor

@nealmcb nealmcb left a comment

Choose a reason for hiding this comment

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

Great work - much appreciated!

Could change it as discussed to use only standard Python tools, and document their usage, in the requirements.in file or wherever.

Also, please note the need with Python 3.4 Ubuntu to use different python3.4-venv package name in some cases.

@cjerdonek cjerdonek force-pushed the smoketest-add-requirements-txt branch from 623cf2a to 6e3152c Compare August 19, 2017 18:39
@cjerdonek
Copy link
Author

Thanks for the review, @nealmcb! I've updated the PR to address your comments.

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

Successfully merging this pull request may close these issues.

4 participants