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 #9785

Merged
merged 12 commits into from
Jun 25, 2019
Merged

Add requirements.*.txt #9785

merged 12 commits into from
Jun 25, 2019

Conversation

marstr
Copy link
Member

@marstr marstr commented Jun 24, 2019

This adds requirements.txt files for the complete matrix of OS and Python that we officially support.

This is the result of instantiating new virtualenv/venv on each OS, running a pip install of each distributable in our repository, ensuring that pip check came back clean, and finally running pip freeze redirected to the appropriate location.


pip3 install -r ${REPO_ROOT}/src/azure-cli/requirements.py3.linux.txt

pip3 check
Copy link
Member

Choose a reason for hiding this comment

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

What does a failure look like here? The old script did have a decent amount of amplifying information to make the necessary course of action obvious.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair question! You can see an example of failure here:
https://travis-ci.org/Azure/azure-cli/jobs/549989807#L800-L803

@@ -28,7 +28,7 @@ jobs:
python: 3.6
- stage: precheck
env: PURPOSE='Dependency Check'
script: ./scripts/dependency/check.sh
script: ./scripts/ci/dependency_check.sh
python: 3.6
# - stage: verify
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't there be more edits to the CI where we install the CLI from the requirements file, such as before running tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, there's a family of PRs coming as I update the tests and installers. This was just to get started before I unleash all of those, to make sure we are all on the same page in terms of general approach.

Copy link
Member

Choose a reason for hiding this comment

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

So I think the existing script is more self-documenting in terms of what someone should do to fix the issue. I don't see where it calls out which requirements file is the offending one. Also, is this only being run for Python 3 Linux since that is what the CI runs? What about the other requirements files? How will we know they need to be updated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mind if I address this in a follow-up? I think the points raised here are important, but I think it's more critical that I get started on overhauling the installers to use these dependency lists.

Copy link
Member Author

@marstr marstr Jun 25, 2019

Choose a reason for hiding this comment

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

Tracked in #9795

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

Successfully merging this pull request may close these issues.

2 participants