Skip to content

Conversation

gaelgatelement
Copy link
Contributor

This allows to call commands installed through poetry without going through python -m <module>..

This is used extensively in ESS-Helm repo.

@gaelgatelement gaelgatelement requested a review from a team as a code owner June 30, 2025 10:28
Comment on lines +37 to +41
load-poetry-path:
description: >
Set to "true" to add the path to the poetry executable to the PATH.
required: false
default: "false"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not default to true as a sane default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly wanted to avoid changing the action default behaviour. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Feels like nothing would change for existing consumers of the action if we default to true.

Comment on lines +120 to +123
- name: Load poetry path
run: |
echo "$(poetry env info -p)/bin" >> "${GITHUB_PATH}"
if: "${{ inputs.load-poetry-path == 'true' }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

As an alternative, what about just using poetry run xxx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also works, but third-party scripts might expect the "binary" name to be available without having to go through poetry. (typically the case when installing ansible for example)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that an actual use case yet? Link?

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