Skip to content

Update GHA workflow #30

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

Merged
merged 14 commits into from
Dec 5, 2022
Merged

Update GHA workflow #30

merged 14 commits into from
Dec 5, 2022

Conversation

hmgaudecker
Copy link
Contributor

Changes

Some stuff from recent work over on econ-project-templates.

  • Use checkout@v3 instead of v2 to get rid of deprecation warnings
  • Use micromamba instead of conda-incubator (speed, better interface)

Have a look, maybe useful here, too!

@codecov
Copy link

codecov bot commented Dec 2, 2022

Codecov Report

Merging #30 (c7a6ff6) into main (d1fd493) will not change coverage.
The diff coverage is n/a.

❗ Current head c7a6ff6 differs from pull request most recent head df252e7. Consider uploading reports for the commit df252e7 to get more accurate results

@@           Coverage Diff           @@
##             main      #30   +/-   ##
=======================================
  Coverage   79.34%   79.34%           
=======================================
  Files           4        4           
  Lines          92       92           
=======================================
  Hits           73       73           
  Misses         19       19           
Flag Coverage Δ
end_to_end 79.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tobiasraabe
Copy link
Member

Wow, does it reduce the runtime of the whole workflow from 1:45h to 1:15h? Compare https://github.com/pytask-dev/cookiecutter-pytask-project/actions/runs/3505768446/usage to https://github.com/pytask-dev/cookiecutter-pytask-project/actions/runs/3600786078/usage. Have you seen similar speed-ups?

@tobiasraabe
Copy link
Member

We need to add the same to the workflow in the template.

@hmgaudecker
Copy link
Contributor Author

hmgaudecker commented Dec 2, 2022

Wow, does it reduce the runtime of the whole workflow from 1:45h to 1:15h? Compare https://github.com/pytask-dev/cookiecutter-pytask-project/actions/runs/3505768446/usage to https://github.com/pytask-dev/cookiecutter-pytask-project/actions/runs/3600786078/usage. Have you seen similar speed-ups?

I have not compared in detail, also because too many things in between two working versions changed when it came to the tests themselves.

But I would not be surprised based on local observations if conda requires half a minute more to resolve the environment on a slow machine.

Oh, I was reading minutes instead of hours 😆. Still, comparing individual tests you see that you gain a minute on Ubuntu for the environment generation step, will be much more on other OS's.

@hmgaudecker
Copy link
Contributor Author

hmgaudecker commented Dec 2, 2022

One thing you may want to think about here is to get rid of tox, too (at least in the outer project). It definitely is a helpful tool when you want to make sure a library is working with all kinds of different dependencies. But here the main issue seems to be testing across operation systems, then you need GHA anyhow.

What I view as an issue is that with the current setup, you have 3 different places for conda environments

  • GHA
  • tox
  • environment.yml

Same for the inner project. Left me confused while debugging things on the other templates repo (same here why things suddenly failed in f1b8359... There I am down to one environment specified in the yaml file (plus the same for the inner project).

@tobiasraabe
Copy link
Member

But I would not be surprised based on local observations if conda requires half a minute more to resolve the environment on a slow machine.

Oh, I was reading minutes instead of hours 😆. Still, comparing individual tests you see that you gain a minute on Ubuntu for the environment generation step, will be much more on other OS's.

I think the caching that is saving environments for one day is the main driver here. Especially on windows. And you are right, it would be even faster without tox.

@tobiasraabe
Copy link
Member

I am also not super happy with the tox setup. Here, it prevents caching, and you mentioned another issue. I generally like isolated environments for testing, but does it have many more benefits compared to using the main environment?

@hmgaudecker
Copy link
Contributor Author

I think the caching that is saving environments for one day is the main driver here.

On Ubuntu, that's 10s (27 on first run vs 17 on second AFAICT).

@hmgaudecker
Copy link
Contributor Author

I am also not super happy with the tox setup. Here, it prevents caching, and you mentioned another issue. I generally like isolated environments for testing, but does it have many more benefits compared to using the main environment?

On the CI machine no -- by definition you get a fresh environment by definition (well, gotta the micromamba guys to get the caching right).

I think the main benefit of tox is that you can easily test many different configurations on local machines. Here, GHA gives you the same functionality.

To the extent that you have one environment per project anyhow, I'd say it's good enough to use that also for testing -- especially if you are running the CI pipeline upon the next push.

@tobiasraabe tobiasraabe merged commit ba9e059 into main Dec 5, 2022
@tobiasraabe tobiasraabe deleted the update_gha branch December 5, 2022 00:23
@tobiasraabe tobiasraabe added this to the v1.4.0 milestone Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants