Skip to content

Add caching in TravisCI#1405

Closed
utkbansal wants to merge 15 commits intoMDAnalysis:developfrom
utkbansal:travis-cache
Closed

Add caching in TravisCI#1405
utkbansal wants to merge 15 commits intoMDAnalysis:developfrom
utkbansal:travis-cache

Conversation

@utkbansal
Copy link
Member

Fixes #1394

Changes made in this Pull Request:

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

@kain88-de
Copy link
Member

We only use pip right now for installing griddataformats abut that will be dropped soon. You have to cache miniconda. You can for the time being use your own fork of the ci-helpers and make the appropriate changes that it works nicely with caching.

@kain88-de
Copy link
Member

Another alternative is to move our dependencies from a conda to a pip dependency.

@utkbansal
Copy link
Member Author

utkbansal commented Jun 16, 2017

How might we move to pip, can you elaborate?

We do this to setup our dev environment

mkvirtualenv mdanalysis
pip install numpy
pip install cython
pip install -e package/
pip install -e testsuite/

Nowhere have I ever used conda to setup MDA. Doing the same thing in Travis should also get the dependencies, right?

@kain88-de
Copy link
Member

Push the dependencies into the PIP_DEPS* variable

@utkbansal
Copy link
Member Author

We have CONDA_DEPENDENCIES and CONDA_ALL_DEPENDENCIES, the first one is for the minimal build and the second one for the complete build right?

.travis.yml Outdated
- source ci-helpers/travis/setup_conda_$TRAVIS_OS_NAME.sh
# - source ci-helpers/travis/setup_conda_$TRAVIS_OS_NAME.sh
# additional external tools (Issue #898) -- HOLE
- virtualenv venv --system-site-packages
Copy link
Member

Choose a reason for hiding this comment

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

With this change you need to remove more. The cron type job won't work anymore and the test for old numpy version can't be done either.

Copy link
Member

Choose a reason for hiding this comment

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

Also dependencies like netcdf will be harder to install. You will likely need to add some apt packages.

Copy link
Member Author

@utkbansal utkbansal Jun 16, 2017

Choose a reason for hiding this comment

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

Should I do a sudo pip install <dependencies> then?

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 won't solve the install problem. netcdf has additional compile time dependencies. You have to specify them earlier in the travis config. You can look into the travis docs how to do that.

.travis.yml Outdated
# - source ci-helpers/travis/setup_conda_$TRAVIS_OS_NAME.sh
# additional external tools (Issue #898) -- HOLE
- sudo pip install --upgrade-pip
- sudo pip install $PIP_DEPENDENCIES
Copy link
Member

Choose a reason for hiding this comment

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

you can find in the logs that the sudo fails because you aren't allowed to write. The previous method was ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't understand.

Copy link
Member

Choose a reason for hiding this comment

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

@richardjgowers
Copy link
Member

I think moving to pip might cause more problems than it solves.. Can't we just cache the conda env? It's just a directory somewhere..

@utkbansal
Copy link
Member Author

@kain88-de @richardjgowers Do we want to have a discussion on this?

@kain88-de
Copy link
Member

caching the conda envs will mean we have to change the ci-helper script to handle caching on travis. But that shouldn't be so hard. Basically we skip the installation if the miniconda folder exists.

@utkbansal
Copy link
Member Author

Okay, I'll try conda then.

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

I am not plugged into the discussion so you might have already answered or considered all the things that I note in my comments.

.travis.yml Outdated
- source ci-helpers/travis/setup_conda_$TRAVIS_OS_NAME.sh
# - source ci-helpers/travis/setup_conda_$TRAVIS_OS_NAME.sh
# additional external tools (Issue #898) -- HOLE
- sudo pip install --upgrade pip
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need sudo and why do we need to upgrade pip? Isn't this taken care of by the conda installation?

Avoiding use of sudo is desirable so that we can run on the Travis container-based infrastructure.

.travis.yml Outdated


install:
- sudo apt-get install libssl1.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to install libssl and why do we use sudo here?

Avoiding use of sudo is desirable so that we can run on the Travis container-based infrastructure. For apt you can use Installing Packages on Container Based Infrastructure without requiring sudo.

.travis.yml Outdated

install:
- sudo apt-get install libssl1.0.0
- git clone git://github.com/astropy/ci-helpers.git
Copy link
Member

Choose a reason for hiding this comment

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

Clone git://github.com/astropy/ci-helpers.git (in fact, maybe MDAnalysis should have a clone... – @kain88-de ?) and then pull from the clone with your changes.

Contribute your changes (once working) as PR to upstream.

@kain88-de
Copy link
Member

This doesn't have high priority for me. The times seem to have gone down again and we are at ~5 minutes installation time. That is just a minute slower then before. The caching would still be nice but I don't think it's a issue that reallly has to be solved before moving to pytest/.

@kain88-de
Copy link
Member

doesn't look like we need this right now.

@kain88-de kain88-de closed this Jul 12, 2017
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