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

fix: use virtualenv instead of pipx for renku-python #128

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

ableuler
Copy link
Contributor

@ableuler ableuler commented Sep 8, 2021

Depends on SwissDataScienceCenter/renkulab-docker#187 being released.
Ready to merge.

Comment on lines 38 to 42
source .renku-python/bin/activate ; \
version_string=$(pip show renku | grep Version) ; \
pattern='Version: (.*)$' ; \
[[ $version_string =~ $pattern ]] ; \
currentversion=${BASH_REMATCH[1]} ; \
Copy link
Member

Choose a reason for hiding this comment

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

what's wrong with renku --version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't you see the beauty in that regex...? 🤦
fair point

Copy link
Member

Choose a reason for hiding this comment

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

well maybe @Panaetius had some reason to not use it in the first place either...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe it simply predates the introduction of renku --version?

Copy link
Member

Choose a reason for hiding this comment

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

no that's been around forever

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know what renku --version returns for non-released versions? I guess @Panaetius will tell us :)

Copy link
Member

Choose a reason for hiding this comment

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

something like this: 0.16.1.dev16+g78fad896 - see the long regex at the bottom of the Dockerfile...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed, renku --version and the version displayed by pip show or pipx list seem to be identical also dev versions. I don't see a reason for not using renku --version then.

@ableuler ableuler marked this pull request as ready for review September 9, 2021 13:49
Copy link
Member

@Panaetius Panaetius left a comment

Choose a reason for hiding this comment

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

This looks great, thank you!

@ableuler ableuler merged commit 2c82f60 into master Sep 9, 2021
@ableuler ableuler deleted the 000-pin-setuptools branch September 9, 2021 21:32
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