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.07] install Conda at startup; enable conda_auto_init in sample conf #2759

Merged
merged 5 commits into from Aug 11, 2016

Conversation

Projects
None yet
6 participants
@martenson
Copy link
Member

commented Aug 9, 2016

  • adjust wording
@jmchilton

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

This installs conda when Galaxy starts up - not before the first time a tool dependency attempts to be installed - I'm not sure this is a good idea - I'd prefer to change this default only if it this conda installation were to occur as part of a dependency install. Thoughts?

@martenson

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2016

@jmchilton are you referring to the change in init.py or am I misunderstanding how it works?

@jmchilton

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

Yeah - your change of that default causes this code https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/tools/deps/resolvers/conda.py#L92 to run when Galaxy starts up by default. I'd prefer Galaxy never access the Internet for instance unless explicitly asked too.

@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

My feeling is that during startup admins expect such changes, like database migrations, tool migrations, wheel updates and so on. During tool installation I would not expect a conda installation and any error could be confusing.

# Following conda dependeny resolution options are experimental as of early
# 2016. These will change the defaults for each conda resolver, but multiple
# resolvers can be configured independently and these options overridden in
# Following Conda dependency resolution options will change the defaults for each Conda resolver,

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Aug 9, 2016

Member

s/Following/The following/

@jmchilton

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

You lost me at "tool migration" but I guess we do update wheels at startup. I don't think we will when we pip install Galaxy those. @natefoo are you okay with Galaxy automatically install conda when it starts up by default?

@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

We did a lot of tool migrations during the last years, replacing tools previously shipped with Galaxy core to TS ones.
If we install Galaxy with pip we can also ship a local miniconda tarball and install this during startup, isn't it?

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

If the consensus is to install conda when the resolver is first used I could do a PR on @martenson's branch with this

@martenson

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2016

@jmchilton

automatically install conda when it starts up by default?

It is not just start up. You have to major-upgrade (release switch) your Galaxy and then start up.
And currently for updating wheels you pull latest and then it forces you to download wheels.
And for first ever startup you have to fetch dozens of wheels and there is no easy workaround.

@martenson

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2016

If we delay the Conda installation attempt until the resolver is first used we will also force admins to try installing a (conda-only-requirement) tool to make sure Conda installs fine. Wouldn't they benefit from this information during startup when they are setting things up? Surely the Conda installation can fail under some circumstances...

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

Wouldn't they benefit from this information during startup when they are setting things up?

That is true ... I think right now it probably makes sense to install conda at startup. IMO we can revisit this decision once pip install galaxy also fetches wheels and the conda install slows things down disproportionately.

@jmchilton

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

I'm -0 on this - I think this is making the assumption that conda will be used - it should be considered optional and the assumption shouldn't be that it will be used automatically. There are enough +1s that this will be merged anyway and I'm okay with that - I see the upsides.

@martenson

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2016

@jmchilton Do we want to motivate deployers to move towards Conda? Do we want to troubleshoot less failing TS package recipes installations? Do we want to test this on variety of deployments?

edit: These I see as benefits of merging this. Don't we ultimately want to have Conda as the first entry in the dependency_resolver.conf?

@martenson martenson changed the title [16.07] enable conda_auto_init in sample, adjust wording [16.07] install Conda at startup; enable conda_auto_init in sample conf Aug 9, 2016

@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

Traditional Tool dependencies are also installed by default and we want to enable reproducible workflows, which is only possible if the dependencies are also tracked and reproducible on other systems. We agreed to go with conda so we should activate it on all levels by default. If someone wants to use modules or any other manual system, it is easy enough to deactivate these settings.

Make it easy and reproducible by default! 👍

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Aug 9, 2016

The startup test on OSX probably fails because it's timing out during the conda install ... or because the prefix may be ridiculously long on travis. @martenson can you include martenson#15 ?

@natefoo

This comment has been minimized.

Copy link
Member

commented Aug 10, 2016

I am +1 on installing conda at startup time, and I agree that this should be done by default now, since it is our preferred dependency resolver going forward, and TS build recipes will no longer be required, even for IUC/devteam tools. TS tools will eventually stop working if we don't make this the default.

I also prefer to have it done at startup time rather than "some time in the future when Galaxy is running"

mvdbeek and others added some commits Aug 10, 2016

@martenson

This comment has been minimized.

Copy link
Member Author

commented Aug 10, 2016

PR from @mvdbeek was added to adjust the starting wait (which was failing on OSX travis)

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Aug 10, 2016

👍 once the tests pass.

@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 11, 2016

Green, Green. Thanks to all!

@bgruening bgruening merged commit a7be24e into galaxyproject:release_16.07 Aug 11, 2016

4 checks passed

api test Build finished. 224 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 111 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 582 tests run, 0 skipped, 0 failed.
Details

@bgruening bgruening deleted the martenson:conda_config branch Aug 11, 2016

jmchilton added a commit that referenced this pull request Aug 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.