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.

Sign up for GitHub

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

Jump to bottom

Stop using 'pip search' for ensure => latest #434

Merged
merged 1 commit into from
Nov 26, 2018
Merged

Stop using 'pip search' for ensure => latest #434

merged 1 commit into from
Nov 26, 2018

Conversation

gdubicki
Copy link
Member

@gdubicki gdubicki commented Oct 5, 2018 •
edited
Loading

Fixes #433

@gdubicki gdubicki mentioned this pull request Oct 5, 2018
Closed
@gdubicki gdubicki force-pushed the dont_use_pip_search branch 4 times, most recently from f36dcae to c67438d Compare October 8, 2018 07:46
@gdubicki gdubicki changed the title WIP: Stop using 'pip search' Stop using 'pip search' for ensure => latest Oct 8, 2018
@gdubicki
Copy link
Member Author

Note that this PR causes $url to be ignored when using $ensure = latest - only $pkgname is used.

I am not sure if that is wrong though because I doubt that the previous code did work correctly with $url..

What do you think, @danquack ?

bastelfreak
bastelfreak approved these changes Nov 2, 2018
View reviewed changes
Copy link
Member

@bastelfreak bastelfreak left a comment

Choose a reason for hiding this comment

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

LGTM, is there somebody else that could review this?

@bastelfreak
Copy link
Member

@gdubicki can you check the failed travis tests?

@gdubicki
Copy link
Member Author

gdubicki commented Nov 2, 2018

@gdubicki can you check the failed travis tests?

I did, but I can't find the cause.. I am using Centos 7 in my infra and the code runs fine on many nodes for a few weeks now. I have spin up a vm with Debian 9 to test why unless seems not to work the way it should, but commands I am trying give me the same results as on Centos 7...

I don't really have much experience with tiny differences of Python-related stuff behaviour on various distros.. Do you have any ideas why it may be failing?

PS I will of course squash the commits at the end.

@gdubicki gdubicki mentioned this pull request Nov 2, 2018
Closed
bastelfreak
bastelfreak reviewed Nov 2, 2018
View reviewed changes
manifests/dotfile.pp Outdated Show resolved Hide resolved
@bastelfreak
Copy link
Member

This should be rebased after #446 got merged

Stop using 'pip search' for ensure => latest
69dea59 
because it does not work properly with devpi
as own repo/PyPi mirror and is being considered
to be deprecated in pip
@gdubicki
Copy link
Member Author

gdubicki commented Nov 5, 2018

Ready for final review, @bastelfreak @danquack .

@ThiefMaster
Copy link

Wouldn't it be cleaner to use pip's JSON API and get the latest version from there? Check e.g. https://pypi.org/pypi/click/json - getting the "version" from there seems like a better option than relying on certain output when installing an invalid version..

@gdubicki
Copy link
Member Author

I don't like this method too, but I think that as of now this is how you do it in the python world - see https://stackoverflow.com/q/4888027/2693875 . JSON API looks great on pypi but it's not available when you are using pypi mirror, like devpi, which is one of the problems with #433, which this PR fixes, @ThiefMaster .

@gdubicki
Copy link
Member Author

Can we merge this now, after #446 got merged @bastelfreak @danquack ?

@bastelfreak bastelfreak merged commit 6306c13 into voxpupuli:master Nov 26, 2018
@bastelfreak bastelfreak added the bug Something isn't working label Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bastelfreak bastelfreak bastelfreak approved these changes

@thaiphv thaiphv thaiphv left review comments

@danquack danquack danquack left review comments

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gdubicki @bastelfreak @ThiefMaster @thaiphv @danquack