Skip to content

Use 3 build stages for Travis#1349

Merged
bgruening merged 4 commits intogalaxyproject:masterfrom
nsoranzo:travis
Jun 8, 2017
Merged

Use 3 build stages for Travis#1349
bgruening merged 4 commits intogalaxyproject:masterfrom
nsoranzo:travis

Conversation

@nsoranzo
Copy link
Copy Markdown
Member

@nsoranzo nsoranzo commented Jun 8, 2017

The 3 stages are:

  • lint (1 job) for flake8 and planemo shed_lint
  • test (3 jobs) for planemo test
  • deploy (1 job) for planemo shed_update to TTS and MTS

Also:

  • Cache pip files
  • Use --conda_auto_install option of planemo test instead of running
    planemo conda_install beforehand. This will install only the tool
    dependencies required by the tools tested in the job

xref. #1316 (comment)

Fix #1308.

@peterjc
Copy link
Copy Markdown
Contributor

peterjc commented Jun 8, 2017

This is in the order test, lint, deploy whereas I would have used lint, test, deploy.

i.e. Lint first as it is fairly quick and should fail quickly avoiding often wasted CPU time on the main tests.

@peterjc
Copy link
Copy Markdown
Contributor

peterjc commented Jun 8, 2017

Looking at the .travis.yml file, I would have expected lint first - there is something odd about the build numbers on https://travis-ci.org/galaxyproject/tools-iuc/builds/240756115 too - perhaps a Travis question?

  • Test
    • 4537.1 CHUNK=0
    • 4537.2 CHUNK=1
    • 4537.3 CHUNK=2
    • 4537.5 CHUNK=0
  • Lint
    • 4537.4
  • Deploy
    • 4537.6

Where does job 4537.5 come from? It looks like a duplicate of 4537.1 using CHUNK=0

@peterjc
Copy link
Copy Markdown
Contributor

peterjc commented Jun 8, 2017

Is it worth trying putting the environment variable list only under the test stage?

env:
  - CHUNK=0
  - CHUNK=1
  - CHUNK=2

Also, given the relative length of the test's before_install, install and script would it be clearer to define them directly under the test stage?

The 3 stages are:
- lint (1 job) for `flake8` and `planemo shed_lint`
- test (3 jobs) for `planemo test`
- deploy (1 job) for `planemo shed_update` to TTS and MTS

Also:
- Cache pip files
- Use `--conda_auto_install` option of `planemo test` instead of running
  `planemo conda_install` beforehand. This will install only the tool
  dependencies required by the tools tested in the job

xref. galaxyproject#1316 (comment)
@nsoranzo
Copy link
Copy Markdown
Member Author

nsoranzo commented Jun 8, 2017

@peterjc

This is in the order test, lint, deploy whereas I would have used lint, test, deploy.

lint, test, deploy is what I'm trying to achieve as explained in the PR description.

there is something odd about the build numbers

The interaction between build stages and the build matrix is far from ideal at the moment, because test is considered a special build stage which always run first if you have a build matrix, see travis-ci/beta-features#11.
The only way I've found to have the lint stage first is to embed (and demultiplex, see next comment) the build matrix of the test stage inside the jobs: section. It seems to work in the rebased commit now.

Is it worth trying putting the environment variable list only under the test stage?

That's not possible yet, see travis-ci/beta-features#11.

Also, given the relative length of the test's before_install, install and script would it be clearer to define them directly under the test stage?

Since I had to demultiplex the build matrix, it's best to have before_install, install and script in the common part.

Comment thread .travis.yml
# 'sudo required' will give os 7,5 GB memory, but has slower startup times, as we start a VM instead of a Container
sudo: required
language: python
cache: pip
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks to this line the following:

pip install flake8 flake8-import-order planemo

went from ~32s to ~11s.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is very nice!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's a good tip - I should try cache: pip on my TravisCI setups...

@nsoranzo nsoranzo changed the title [WIP] Use 3 build stages for Travis Use 3 build stages for Travis Jun 8, 2017
nsoranzo added 3 commits June 8, 2017 17:36
If multiple repos were changed, chunking using ci_find_tools may result
in running `planemo test` on a mix of tools from different repos,
which does not work because Planemo expects all test data in the same
directory, see e.g.
https://travis-ci.org/galaxyproject/tools-iuc/jobs/240466613

Also exclude data_managers and packages from the start for the `test`
stage.
Also:
- Remove deprecated `interpreter` attribute of `<command>`
- Use `detect_errors="exit_code"`
- Use single quotes in `<command>`
- Add CDATA to `<command>` and `<help>`
- Add citation
- IUC code style fixes
- Add Python3 compatibility
Copy link
Copy Markdown
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Cool! Haven't played with that by myself. So if @peterjc is ok with it! I'm 💯

Very nice improvement!

Comment thread .travis.yml
env: CHUNK=2

- stage: deploy
before_install: skip
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this skip needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not really, it's just for a cleaner log.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Without this TravisCI will use the top level before_install instructions which we don't need or want to be run (although right now that would be quick and harmless).

Comment thread .travis.yml
# 'sudo required' will give os 7,5 GB memory, but has slower startup times, as we start a VM instead of a Container
sudo: required
language: python
cache: pip
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is very nice!

@peterjc
Copy link
Copy Markdown
Contributor

peterjc commented Jun 8, 2017

The reworked .travis.yml is doing the stages in the intended order, and only defines the CHUNK environment within the test stages - so I'm happy with those changes 👍

(Presumably the other changes are on this branch in part to ensure there is something to test?)

@nsoranzo
Copy link
Copy Markdown
Member Author

nsoranzo commented Jun 8, 2017

Presumably the other changes are on this branch in part to ensure there is something to test?

Yes, in particular that there are more than 1 repo with several tools.

@bgruening
Copy link
Copy Markdown
Member

Ah this is beauty! What would we do without Nicola!
Thanks a bunch!!!

@bgruening bgruening merged commit e2075ca into galaxyproject:master Jun 8, 2017
nsoranzo added a commit to nsoranzo/planemo that referenced this pull request Jun 8, 2017
…s where in subdirs

E.g. in the Travis job:

https://travis-ci.org/galaxyproject/tools-iuc/jobs/240866693

for the pull request:

galaxyproject/tools-iuc#1349

changed_repositories.list should also have contained the repo
`data_managers/data_manager_bowtie_index_builder`
@nsoranzo
Copy link
Copy Markdown
Member Author

nsoranzo commented Jun 8, 2017

And also the deploy stage worked (apart from a Planemo bug which will be fixed by galaxyproject/planemo#688)

https://travis-ci.org/galaxyproject/tools-iuc/builds/240884130

@nsoranzo
Copy link
Copy Markdown
Member Author

nsoranzo commented Jun 8, 2017

@bgruening Maybe that's something you can mention in the IUC talk at GCC2017 ;)

@bgruening
Copy link
Copy Markdown
Member

Sure, this will be in the talk, but maybe you will give the talk :)

Thanks @nsoranzo!

@nsoranzo
Copy link
Copy Markdown
Member Author

nsoranzo commented Jun 8, 2017

@bgruening I'm the session 3 chair!

@jmchilton
Copy link
Copy Markdown
Member

Use --conda_auto_install option of planemo test instead of running
planemo conda_install beforehand.

This is the default in the latest versions of Conda I think - you shouldn't need to specify it.

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