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

Make st2client setup.py requirements consistent #4209

Merged
merged 13 commits into from
Jul 9, 2018
Merged

Make st2client setup.py requirements consistent #4209

merged 13 commits into from
Jul 9, 2018

Conversation

LindsayHill
Copy link
Contributor

Installing st2client using pip install st2client has missing dependencies. It is unusable until you run pip install argcomplete pytz jsonschema prompt_toolkit python-editor.

This PR makes the setup.py requirements consistent with those in requirements.txt.

Fixes #3367

@bigmstone
Copy link
Contributor

@Kami Is the reason you disabled pulling from requirements.txt in #2423 still valid or can we re-enable that? It'd be a better fix than trying to manually keep st2client in sync.

@LindsayHill
Copy link
Contributor Author

FWIW, I would be much happier about having this auto-synced. I'm just trying to deal with some immediate pain, but a 'proper' fix would be welcome.

@Kami
Copy link
Member

Kami commented Jun 28, 2018

We fixed dist_utils.py so it also works with pip 10.0.0 so I would be OK with reading from requirements.txt again if we can indeed verify it works correctly.

Still need to verify it works with various versions of pip (pre 9, 9, and 10). I believe in the past there were issues with older versions of pip.

@Kami Kami added this to the 2.9.0 milestone Jun 28, 2018
@Kami
Copy link
Member

Kami commented Jun 28, 2018

I did some digging in and testing - we now install pip 9.0.1 inside st2 and pack virtual environments so I believe we should be fine.

It doesn't work with very ancient versions of pip, but I just checked and most other software also doesn't support pip < 9.0 so we should be fine.

I will push a change which updates it to use requirements.txt again.

@LindsayHill thanks for opening this PR and having us reevaluate this decision again.

I also tagged this issue with 2.9.0 and will wait with merging this we have a branch for v2.8.

Kami added 4 commits June 28, 2018 10:37
requirements.txt file.

This is now possible since we install pip 9.0.0 in pack and st2
virtual environments.

Keep in mind that this won't work with very old versions of pip
(< 9.0).
of pip where the attribute was called req.url (in newer versions it's
called req.link).
@Kami
Copy link
Member

Kami commented Jun 28, 2018

I pushed various changes which should resolve that - f9455e2, 8c6659c, 79d0d13.

It should work with pip >= 6.0.0, but we really shouldn't support anything < 9.0.0 so if a user complains, we should ask them to upgrade / use more recent version of pip.

@LindsayHill
Copy link
Contributor Author

Thanks for that @Kami. Totally fine for this to be 2.9.0. It's not a 2.8.0 blocker.

@Kami
Copy link
Member

Kami commented Jul 3, 2018

I pushed some more changes / fixes to this branch and confirmed it indeed fixes issue reported in #3367 - #3367 (comment).

@Kami Kami merged commit f279ea1 into master Jul 9, 2018
@arm4b arm4b deleted the setup_reqs branch July 9, 2018 15:06
@Kami Kami mentioned this pull request Jul 16, 2018
@Kami Kami modified the milestones: 2.9.0, 2.8.1 Jul 16, 2018
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.

3 participants