Skip to content

Update pip dependencies, fix conflicting dependency version, ensure there are no conflicting dependencies #4819

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

Merged
merged 14 commits into from
Nov 20, 2019

Conversation

Kami
Copy link
Member

@Kami Kami commented Nov 19, 2019

This pull request includes the following changes:

  1. Update cryptography dependency to v2.8 since version of requests we use depends on that version. As per Master currently broken because of dependency conflict between requests and cryptography #4818 (comment), that is not actually a fatal issue since pip just prints a warning and doesn't fail on install, but it's still good to get it sorted out.
  2. Update various dependencies to latest versions (jinja2, requests and pip, setuptools and pbr for tests)
  3. Make sure versions in test-requirements.txt are in sync with versions in requirements.txt
  4. Run pip-conflict-checker as part of make requirements step. This command will fail and exit with non-zero if there are any conflicting dependency versions.

TODO

  • Changelog entries

Resolves #4818.

@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Nov 19, 2019
@Kami Kami added this to the 3.2.0 milestone Nov 19, 2019

# Generate all requirements to support current CI pipeline.
$(VIRTUALENV_DIR)/bin/python scripts/fixate-requirements.py --skip=virtualenv,virtualenv-osx -s st2*/in-requirements.txt contrib/runners/*/in-requirements.txt -f fixed-requirements.txt -o requirements.txt

# Remove any *.egg-info files which polute PYTHONPATH
rm -rf *.egg-info*
Copy link
Member Author

Choose a reason for hiding this comment

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

We should have done that before.

We add repo root to PYTHONPATH in virtualenv we create which means if you run python setup.py install or similar in st2client/ directory, it will create egg-info directory for that Python package which will mask actual st2client/ directory in PYTHONPATH.

@Kami
Copy link
Member Author

Kami commented Nov 19, 2019

I confirmed conflict checker tool works correctly. Here is an example if we use kombu==4.6.6 and amqp==2.5.1.

# Verify there are no conflicting dependencies
virtualenv/bin/pipconflictchecker
--------------------------------------------------
 Conflicts Detected
--------------------------------------------------
 - amqp(2.5.1) kombu(>=2.5.2,<2.6)
Makefile:437: recipe for target 'requirements' failed

It will return an error since that version of kombu request amqp >= 2.5.2.

@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/M PR that changes 30-99 lines. Good size to review. labels Nov 19, 2019
pyrabbit
# Since StackStorm v2.8.0 we now use cryptography instead of keyczar, but we still have some tests
# which utilize keyczar and ensure new cryptography code is fully compatible with keyczar code
# (those tests only run under Python 2.7 since keyczar doesn't support Python 3.x).
# See https://github.com/StackStorm/st2/pull/4165
python-keyczar
git+https://github.com/Kami/pip-conflict-checker.git@fix_pip_issue#egg=pip-conflict-checker
Copy link
Member Author

Choose a reason for hiding this comment

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

Just FYI - that's the change we need - ambitioninc/pip-conflict-checker#14.

I can also push that fork to StackStorm org and use that version until it's merged upstream.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here we go - febde46.

@arm4b
Copy link
Member

arm4b commented Nov 20, 2019

  • 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependency size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Master currently broken because of dependency conflict between requests and cryptography
3 participants