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(ci): fix bad use of poetry groups + update to Python 3.12 in publish CI #104

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

cyclimse
Copy link
Collaborator

@cyclimse cyclimse commented Jul 22, 2024

Summary

What's changed?

  • Stop using "main" as the default group in the custom "setup-poetry" action

Why do we need this?

  • Fix the CI, which is blocking the deployment of release v2.0.1

  • I previously relied on a strange behavior, when parsing flags which were lists of strings, poetry would ignore if an element was empty. It now considers this a valid input, so when passing "--only main,", it identifies two groups, main and ``. The latter is not a valid group, so it fails.

  • TBH I'm not sure why I chose this approach as opposed to just using --with, I think there was a specific case where we only needed a non-main dependency group, so I chose this.

How have you tested it?

Checklist

  • I have reviewed this myself
  • There is a unit test covering every change in this PR
  • I have updated the relevant documentation

Details

@cyclimse cyclimse requested review from norbjd and Bemilie July 22, 2024 09:08
@cyclimse cyclimse self-assigned this Jul 22, 2024
@norbjd
Copy link

norbjd commented Jul 22, 2024

LGTM, difficult to say if it will work without releasing, but since it's a copy of what's done on https://github.com/scaleway/dagster-scaleway, and since it already worked there (https://github.com/scaleway/dagster-scaleway/actions/runs/8813500431), I'd say it should work too 😅

@cyclimse cyclimse merged commit 451f6ca into main Jul 22, 2024
6 checks passed
@cyclimse cyclimse deleted the fix/ci-poetry-groups-parsing branch July 22, 2024 12:47
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.

3 participants