search

nf-core / tools

Python package with helper tools for the nf-core community.
https://nf-co.re
MIT License
238 stars 188 forks source link

Proposal for new travis.yml configuration #193

Closed matq007 closed 5 years ago

matq007 commented 5 years ago

I have been thinking a lot about this and I've come up with this solution for how to make a travis.yml better.

Problem

Right now, whenever a new pull request is submitted, travis is forced to pull the pipeline image, tag it as latest and run the pipeline. This means that if a developer made some change in Dockerfile these changes will be ignored when testing. This is also my case scenario where the dev Dockerfile has been completely rewritten compared to the master Dockerfile.

Solution on dev branch

The workflow should be this instead:

  1. Pull nfcore/base image
  2. Build the Dockerfile that is in the PR

Solution on master branch

  1. Pull nfcore/base image
  2. Build the Dockerfile that is in the PR
  3. Tag the image as latest and push it

Ofcourse the lint logic would have to be changed also.

sven1103 commented 5 years ago

Thanks a lot @matq007 for your suggestion and thoughts. For the dev branch I think your solution would be quite nice indeed.

However for the master branch, I would rather pull from Dockerhub, and make sure that a working container is available on remote resources. We have to make sure that the tests mimic the real world scenario of the pipeline execution as close as possible, this includes container pull, before we prepare releases.

Does this make sense? Feel free to comment ;)

Best, Sven

matq007 commented 5 years ago

You can't build Dockerfile on dev and ignore it on master. Eventually, the dev branch will be merged to master meaning that the dev Dockerfile becomes new master Dockerfile. If the changes coming from dev to master are too big your tests will fail eventually somewhere. That's why I proposed to change to workflow to actually build the Dockerfile and set that as a manifest for the pipeline version not the other way around.

Example scenario: The rnafusion pipeline went from having one huge image for all tools to image per tool. So right now, the lint is forcing me to download the master Dockerfile which I am not interested in. It's outdated and is just a huge image for me that even has wrong python version. In my scenario, Travis would just build the Dockerfile and if all the tests and lint passed, it would just tag it and deploy straight as a new latest image (or version it by release number).

sven1103 commented 5 years ago

Ah, meaning that we push to Dockerhub directly after successful build?

matq007 commented 5 years ago

Yes, we only push once a PR from dev to master passes lint and tests, which would mean a new version of the pipeline is introduced to the world.

sven1103 commented 5 years ago

yup, does make sense to me now.

ewels commented 5 years ago

I think we used to do this, but Travis times out on the builds so it doesn't work..?

apeltzer commented 5 years ago

It gets problematic as soon as we reach 45min. I believe we can extend build times a bit, but apart from travis_wait <n> (n<30) minutes, we won't get much more unfortunately. If tests are already running for lets say 25 minutes and we need > 20 Minutes to push to DockerHub, that won't work.

Also, this means that we'd need to configure Docker Hub keys etc, making the setup of Travis a bit more complicated in general (though not much tbh).

ewels commented 5 years ago

...waits for @matq007 to suggest that we use an on-site jenkins server running on an old repurposed HiSeq server..

..we want to keep CI public and free ideally :) Yes Circle CI maybe could work but it's a hassle to switch and I'm not sure if it's worth it.. Thoughts?

matq007 commented 5 years ago

My guess would be that new pipelines in the future will be more and more demanding for free CI anyway.

ewels commented 5 years ago

Absolutely - I suspect that we will have to strip away more and more of the actual pipeline runs themselves and be left with just the linting and syntax checks (eg. nextflow run xxx --help).

I can definitely see a homebrew heavy-duty jenkins CI server running in parallel for proper run tests, I'm just not super keen on making the entire project rely on such a setup.

ewels commented 5 years ago

Closing this issue for now - the idea is sound, but I don't think that it will work on Travis as things stand. Can come back to it if/when we have a more capable CI setup such as a local Jenkins server.