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

[16.01] Fix virtual env handling broken with #1847. #1890

Merged
merged 3 commits into from
Mar 15, 2016

Conversation

jmchilton
Copy link
Member

Thanks for the bug report @natefoo.

@nsoranzo
Copy link
Member

nsoranzo commented Mar 8, 2016

Should we add tests to Travis to check that these commands works, like we do for run.sh ?

@jmchilton
Copy link
Member Author

I would say yes, but the existing travis check for run.sh is kind of noisy right? Does anyone know why it fails semi-frequently?

@dannon
Copy link
Member

dannon commented Mar 8, 2016

Kinda nitpicky, but now it prints twice:

[yoplait@anvil:~/galaxy] sh run_reports.sh                                                                                                                                                                                     29c78e9 ✭
Activating virtualenv at .venv
Activating virtualenv at .venv

@nsoranzo
Copy link
Member

nsoranzo commented Mar 8, 2016

@jmchilton Is it? I haven't noticed many failures, but maybe somebody re-run the builds superfast.

@jmchilton
Copy link
Member Author

I re-ran like 3 or 4 times - but admittedly it was all within a couple days about a week ago. Maybe it was some transient wheel or Travis problem that has been resolved.

@jmchilton
Copy link
Member Author

@dannon good point, I've removed the print statements added with this PR.

@nsoranzo
Copy link
Member

nsoranzo commented Mar 8, 2016

@dannon Actually also run.sh prints it twice, one for scripts/common_startup.sh and one for itself.

@natefoo
Copy link
Member

natefoo commented Mar 8, 2016

It always printed it twice.

@dannon
Copy link
Member

dannon commented Mar 8, 2016

Sounds like we need to fix that in one more spot, then :)

Anyway, +1 for this change to 16.01.

@nsoranzo
Copy link
Member

nsoranzo commented Mar 9, 2016

I think the double print is useful when debugging problems with virtual envs to check that scripts/common_startup.sh is sourcing the same env of run.sh . See also my next comment.

then
. "$GALAXY_VIRTUAL_ENV/bin/activate"
fi

Copy link
Member

Choose a reason for hiding this comment

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

This code must go AFTER ./scripts/common_startup.sh --skip-samples, which will create the .venv directory if it does not exist.

@jmchilton
Copy link
Member Author

I've modified the order as requested by @nsoranzo.

@natefoo
Copy link
Member

natefoo commented Mar 15, 2016

Merging this but we should probably switch to sourcing, further development in #1899

natefoo added a commit that referenced this pull request Mar 15, 2016
[16.01] Fix virtual env handling broken with #1847.
@natefoo natefoo merged commit 00a7c3a into galaxyproject:release_16.01 Mar 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants