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

Fix: Use correct version of Python for poetry tests #208

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

alexdewar
Copy link
Collaborator

@alexdewar alexdewar commented Jan 7, 2025

Description

This PR changes the GitHub workflows from using the setup-poetry action to manually installing poetry via pipx instead. This fixes an issue we're seeing on the Windows runner where the runner's system version of Python is being used by poetry rather than the one we specify (i.e. 3.12).

Interestingly, we had already updated this in the workflows for the template (i.e. the ones which will end up in users' repos) but must have forgotten to do the same for this repo's workflows.

Type of change

  • Documentation (non-breaking change that adds or improves the documentation)
  • New feature (non-breaking change which adds functionality)
  • Optimization (non-breaking, back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (whatever its nature)

Key checklist

  • All tests pass (eg. python -m pytest)
  • Pre-commit hooks run successfully (eg. pre-commit run --all-files)

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added or an issue has been opened to tackle that in the future. (Indicate issue here: # (issue))

This fixes a bug whereby the runner's system Python was used instead of the version we want.

Also add caching for poetry packages.
@alexdewar alexdewar force-pushed the ci-install-poetry-with-pipx branch from 0c09d9a to da402bc Compare January 7, 2025 10:11
@alexdewar alexdewar marked this pull request as ready for review January 7, 2025 10:23
uses: abatilo/actions-poetry@v3.0.0
with:
poetry-version: 1.2.2
cache: pip
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not want poetry here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point. As it stands it will cache the packages used by this project (i.e. cookiecutter and pytest) but not the ones used in the generated template project. I did try setting this value to poetry but it complained there was no poetry.lock in the root folder 😞

Copy link
Contributor

Choose a reason for hiding this comment

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

I think using pip for cache is fine here. There are two different environments being installed, the poetry one is the one that the specification for is build dynamically by cookiecutter within this workflow

Copy link
Contributor

@AdrianDAlessandro AdrianDAlessandro left a comment

Choose a reason for hiding this comment

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

Looks sensible, I do wonder what change in the setup-poetry action caused this to fail. It might be worth checking because it could be better than manually using pipx

uses: abatilo/actions-poetry@v3.0.0
with:
poetry-version: 1.2.2
cache: pip
Copy link
Contributor

Choose a reason for hiding this comment

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

I think using pip for cache is fine here. There are two different environments being installed, the poetry one is the one that the specification for is build dynamically by cookiecutter within this workflow

Copy link
Contributor

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

This looks fine to me. It is something we had changed in other places, so merely an oversight not changing it also here.

@alexdewar alexdewar merged commit d472d15 into main Jan 8, 2025
9 checks passed
@alexdewar alexdewar deleted the ci-install-poetry-with-pipx branch January 8, 2025 12:05
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.

4 participants