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

Use faster ItsDrike/setup-poetry in favor of HassanAbouelela/actions/setup-python #487

Merged
merged 4 commits into from
Feb 20, 2023

Conversation

ItsDrike
Copy link
Member

@ItsDrike ItsDrike commented Feb 16, 2023

This PR updates all workflows that require dependency installation with poetry to use ItsDrike/setup-poetry which is significantly faster to the current HassanAbouelela/actions/setup-python.

For an in-depth description of what's changed, feel free to check py-mine/mcproto#12, which was a PR with same goal (moving from HassanAbouelela/actions/setup-python to ItsDrike/setup-poetry). It's description includes a performance analysis, comparing the actual times action runs took between these two, and mentions some caveats that switch has.

For a quick overview, here's the important things:

Improvements this switch brings us

  • Installing (latest, or specified version of) poetry globally, using the official installer
  • Standalone caching of the global poetry installation
  • Standalone caching of the dependencies installation
    • Restore-key based caching, allowing cache restore even without exact key match, making version updates much faster
  • Functioning caching for windows (with HassanAbouelela/actions/setup-python, the windows caching doesn't seem to be working at all)
  • Caches are stored within the ItsDrike/setup-poetry action, ahead of time, not as post hooks, meaning whenever dependency installation step succeeds, caches will be created, even if the action fails in some step later on.

Extra enhancements

  • The original HassanAbouelela/actions/setup-python was also caching pre-commit environments (at least for linux where caching actually worked), which the ItsDrike/setup-poetry is not doing. For that reason, an additional step was added specifically to cache the pre-commit environment in the validation workflow.
  • An extra step for caching pytest's cache directory was added, speeding up the test running in tox-test workflow.

Note

As this change will now be installing poetry globally, rather than as one of the project's dependencies, making it a part of the project's venv binaries, to make tox work with poetry installations in the CI, adding poetry dependency to the workflow-tox group was necessary.

However this doesn't affect end users nor developers, as this dependency group is only used within the workflows.

@ItsDrike ItsDrike added type: enhancement Improvement to an existing feature area: CI Related to continuous intergration and deployment labels Feb 16, 2023
@ItsDrike ItsDrike changed the title Use faster ItsDrike/setup-poetry in favor of HassanAboulela/actions/setup-python Use faster ItsDrike/setup-poetry in favor of HassanAbouelela/actions/setup-python Feb 16, 2023
@ItsDrike ItsDrike marked this pull request as ready for review February 16, 2023 16:50
.github/workflows/tox-test.yml Outdated Show resolved Hide resolved
.github/workflows/validation.yml Show resolved Hide resolved
.github/workflows/tox-test.yml Outdated Show resolved Hide resolved
poetry.lock Show resolved Hide resolved
.github/workflows/tox-test.yml Outdated Show resolved Hide resolved
Having this cache saves about 200ms from the pytest run, however the
cache restoring alone often takes longer, especially on windows, meaning
it actually slows the CI down on average.
@PerchunPak PerchunPak mentioned this pull request Feb 18, 2023
Copy link
Member

@PerchunPak PerchunPak left a comment

Choose a reason for hiding this comment

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

Thanks!

@kevinkjt2000 kevinkjt2000 merged commit 8b90a58 into master Feb 20, 2023
@kevinkjt2000 kevinkjt2000 deleted the switch-setup-poetry-action branch February 20, 2023 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: CI Related to continuous intergration and deployment type: enhancement Improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants