BUILD-8956 Extract config-poetry action from build-poetry#292
Conversation
Add a standalone config-poetry composite action that fetches Vault credentials, configures JFrog CLI and Poetry for Repox, caches Poetry dependencies, and sets BUILD_NUMBER. Refactor build-poetry to consume config-poetry the same way build-npm uses config-npm.
Agentic Analysis: Early ResultsAgentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action. 1 issue(s) found across 1 file(s):
Analyzed by SonarQube Agentic Analysis in 3.3 s |
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| - name: Set local action paths | ||
| id: set-path | ||
| if: steps.config-poetry-completed.outputs.skip != 'true' | ||
| shell: bash | ||
| run: | | ||
| echo "::group::Fix for using local actions" | ||
| echo "GITHUB_ACTION_PATH=$GITHUB_ACTION_PATH" | ||
| echo "github.action_path=${{ github.action_path }}" | ||
| ACTION_PATH_CONFIG_POETRY="${{ github.action_path }}" | ||
| host_actions_root="${{ inputs.host-actions-root }}" | ||
| if [[ -z "$host_actions_root" ]]; then | ||
| host_actions_root="$(dirname "$ACTION_PATH_CONFIG_POETRY")" | ||
| else | ||
| ACTION_PATH_CONFIG_POETRY="$host_actions_root/config-poetry" | ||
| fi |
There was a problem hiding this comment.
⚠️ Bug: mise restore runs in wrong dir when working-directory != '.'
In config-poetry/action.yml, the mise.local.toml file and the MISE_BACKUP temp dir are created by the "Set local action paths" step (lines 47-77), which has NO working-directory, so they live in the GitHub workspace root. But the "Configure Poetry authentication" step (line 120) sets working-directory: ${{ inputs.working-directory }} and then runs the restore logic:
rm mise.local.toml
mv "$MISE_BACKUP"/* ... ./ ...
When working-directory is not ., this runs inside the subdirectory:
rm mise.local.tomlfails because the file is in the workspace root, not the subdir. GitHub Actions runs bash steps with-e, so the step fails and breaks the build.- Even if it didn't fail, the user's original mise files would be restored into the subdirectory (
./) instead of the workspace root, leaving the root in an inconsistent state with config-poetry'smise.local.tomlnever removed.
Note this diverges from config-npm/action.yml, whose "Configure NPM authentication" step (line 101-114) does the same backup/restore but does NOT set working-directory, keeping creation and restore in the same (root) directory. build-poetry exposes a working-directory input and passes it through, so any consumer using a non-root working directory will hit this regression (previously build-poetry never rm'd the file).
Fix: keep working-directory only for the poetry_config.sh invocation (which needs the project dir) and perform the mise restore relative to the workspace root, or split the restore into a separate step without working-directory (matching config-npm).
Run poetry_config.sh in the working-directory, but restore mise files in the workspace root (no working-directory) and guard with always() so the backup is restored even if poetry_config.sh fails.:
- name: Configure Poetry authentication
if: steps.config-poetry-completed.outputs.skip != 'true'
shell: bash
working-directory: ${{ inputs.working-directory }}
env:
ARTIFACTORY_URL: ${{ format('{0}/artifactory', inputs.repox-url) }}
ARTIFACTORY_USERNAME: ${{ fromJSON(steps.secrets.outputs.vault).ARTIFACTORY_USERNAME }}
ARTIFACTORY_ACCESS_TOKEN: ${{ fromJSON(steps.secrets.outputs.vault).ARTIFACTORY_ACCESS_TOKEN }}
ARTIFACTORY_PYPI_REPO: ${{ inputs.artifactory-pypi-repo }}
run: $ACTION_PATH_CONFIG_POETRY/poetry_config.sh
- name: Restore mise files
if: always() && steps.config-poetry-completed.outputs.skip != 'true'
shell: bash
run: |
echo "::group::Restore mise files"
rm -f mise.local.toml
mv "${{ steps.set-path.outputs.MISE_BACKUP }}"/* "${{ steps.set-path.outputs.MISE_BACKUP }}"/.* ./ 2>/dev/null || true
rmdir "${{ steps.set-path.outputs.MISE_BACKUP }}" 2>/dev/null || true
echo "::endgroup::"
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| echo "::group::Backup mise files to configure Poetry without interference" | ||
| mise_backup=$(mktemp -d) | ||
| echo "MISE_BACKUP=$mise_backup" >> "$GITHUB_OUTPUT" | ||
| mv mise.* .mise.* mise/ .mise/ .tool-versions "$mise_backup/" 2>/dev/null || true | ||
| cp "$ACTION_PATH_CONFIG_POETRY/mise.local.toml" mise.local.toml | ||
| echo "::endgroup::" |
There was a problem hiding this comment.
💡 Bug: mise files not restored if a step fails before restore
In config-poetry/action.yml, the "Set local action paths" step moves the user's real mise files into a temp backup and substitutes config-poetry's mise.local.toml. The restore happens inside the "Configure Poetry authentication" step, which is NOT marked always() and only runs after several intermediate steps (get-build-number, cache, mise-action, vault). If any of those steps fails, the restore never runs: the user's mise files remain stranded in the temp directory and config-poetry's mise.local.toml is left in place. For standalone config-poetry usage, this leaves the checkout in an inconsistent state for any downstream steps. Consider performing the restore in a dedicated if: always() step (see the suggested fix for the working-directory finding, which also addresses this).
Was this helpful? React with 👍 / 👎
Code Review
|
| Auto-apply | Compact | Unblock |
|
|
|
Was this helpful? React with 👍 / 👎 | Gitar
There was a problem hiding this comment.
Pull request overview
This PR introduces a new config-poetry composite action to set up Poetry + Repox (JFrog) authentication, caching, and build-number management, and refactors build-poetry to delegate that setup to config-poetry (similar to the existing config-npm / build-npm split).
Changes:
- Added
config-poetrycomposite action andpoetry_config.shto configure JFrog + Poetry auth and exportPOETRY_HTTP_BASIC_REPOX_*viaGITHUB_ENV. - Refactored
build-poetryto stop configuring Repox for dependency install, and instead callconfig-poetry. - Added ShellSpec coverage for
config-poetry/poetry_config.sh, updatedbuild-poetrytests, and includedconfig-poetryin kcov include patterns.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
config-poetry/action.yml |
New composite action to configure build number, caching, and Poetry/JFrog auth. |
config-poetry/poetry_config.sh |
New script that configures JFrog CLI + Poetry auth and exports Repox basic-auth env vars. |
config-poetry/mise.local.toml |
Adds mise tool/env config for installing/configuring JFrog CLI for config-poetry. |
build-poetry/action.yml |
Delegates setup to config-poetry and adjusts outputs / vault usage accordingly. |
build-poetry/build.sh |
Removes in-script Repox dependency setup; installs dependencies via plain poetry install. |
spec/config-poetry_spec.sh |
New ShellSpec tests for config-poetry/poetry_config.sh. |
spec/build-poetry_spec.sh |
Updates expectations to reflect moved Repox configuration and renamed install function. |
README.md |
Documents the new config-poetry action and notes build-poetry now calls it. |
.shellspec |
Adds config-poetry to the kcov include pattern. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Cache local Poetry cache | ||
| uses: SonarSource/gh-action_cache@a7d13cdd1c9f097a5f8382ccec463be2831e3dbc # v1.6.0 | ||
| if: steps.config-poetry-completed.outputs.skip != 'true' && inputs.disable-caching == 'false' | ||
| with: | ||
| path: ${{ github.workspace }}/${{ inputs.poetry-cache-dir }} | ||
| key: poetry-${{ runner.os }}-${{ hashFiles('poetry.lock') }} | ||
| restore-keys: poetry-${{ runner.os }}- |
| @@ -64,7 +64,7 @@ outputs: | |||
| value: ${{ steps.build.outputs.project-version }} | |||
There was a problem hiding this comment.
| value: ${{ steps.build.outputs.project-version }} | |
| value: ${{ steps.config.outputs.project-version }} |
?
If moving the set_project_version() function to the config action. See following related comment.
| echo "::group::Install dependencies" | ||
| echo "Installing dependencies..." | ||
| jfrog_poetry_install | ||
| poetry_install_dependencies |
There was a problem hiding this comment.
| poetry_install_dependencies | |
| poetry install |
| poetry_install_dependencies() { | ||
| poetry install | ||
| } |
There was a problem hiding this comment.
We can remove the function, it's empty.
| poetry_install_dependencies() { | |
| poetry install | |
| } |
| - name: Set Artifactory reader role | ||
| if: steps.config-poetry-completed.outputs.skip != 'true' | ||
| shell: bash | ||
| env: | ||
| ARTIFACTORY_READER_ROLE: | ||
| ${{ inputs.artifactory-reader-role != '' && inputs.artifactory-reader-role || | ||
| (github.event.repository.visibility == 'public' && 'public-reader' || 'private-reader') }} | ||
| run: | | ||
| echo "ARTIFACTORY_READER_ROLE=${ARTIFACTORY_READER_ROLE}" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
No need for a separate step, this can be merged with the above set-path step, renamed setup. It also adds values to GITHUB_ENV.
| with: | ||
| path: ${{ github.workspace }}/${{ inputs.poetry-cache-dir }} | ||
| key: poetry-${{ runner.os }}-${{ hashFiles('poetry.lock') }} | ||
| restore-keys: poetry-${{ runner.os }}- |
There was a problem hiding this comment.
The other config actions seem to have an improved cache key that includes the workflow name:
ci-github-actions/config-gradle/action.yml
Lines 193 to 207 in 136be4d
This looks like a debt in the Poetry action.
It will be a breaking change for the existing Poetry caches, but a also a fix to avoid cache collisions.
Either implement now, or consider as out of scope and create a ticket for this specific debt.
| jf config remove repox > /dev/null 2>&1 || true # Ignore inexistent configuration | ||
| jf config add repox --url "${ARTIFACTORY_URL%/artifactory*}" --artifactory-url "$ARTIFACTORY_URL" --access-token "$ARTIFACTORY_ACCESS_TOKEN" | ||
| jf config use repox | ||
| jf poetry-config --server-id-resolve repox --repo-resolve "$ARTIFACTORY_PYPI_REPO" |
There was a problem hiding this comment.
We may want to use --global option
| set_project_version() { | ||
| local current_version release_version digit_count | ||
|
|
||
| if ! current_version=$(poetry version -s); then | ||
| echo "::error title=Invalid project version::Could not get version from Poetry project ('poetry version -s')" >&2 | ||
| echo "$current_version" >&2 | ||
| return 1 | ||
| fi | ||
| export CURRENT_VERSION=$current_version | ||
|
|
||
| release_version=${current_version%".dev"*} | ||
| # In case of 2 digits, we need to add a '0' as the 3rd digit. | ||
| digit_count=$(echo "${release_version//./ }" | wc -w) | ||
| if [[ "$digit_count" -lt 3 ]]; then | ||
| release_version="$release_version.0" | ||
| fi | ||
| if [[ "$digit_count" -gt 3 && $release_version =~ [0-9]+\.[0-9]+\.[0-9]+ ]]; then | ||
| release_version="${BASH_REMATCH[0]}" | ||
| echo "::warning title=Version truncated::Version was truncated to $release_version because it had more than 3 digits" >&2 | ||
| fi | ||
| release_version="$release_version.${BUILD_NUMBER}" | ||
|
|
||
| echo "Replacing version $current_version with $release_version" | ||
| poetry version "$release_version" | ||
| echo "project-version=$release_version" >> "$GITHUB_OUTPUT" | ||
| echo "PROJECT_VERSION=$release_version" >> "$GITHUB_ENV" | ||
| echo "PROJECT_VERSION=$release_version" | ||
| export PROJECT_VERSION=$release_version | ||
| } |
There was a problem hiding this comment.
This should also probably go to the config-poetry action. That's the reason for calling the get-build-number action.
Summary
config-poetrycomposite action for Repox authentication, JFrog CLI setup, Poetry caching, and build number managementbuild-poetryto delegate configuration toconfig-poetry, mirroring theconfig-npm/build-npmsplitconfig-poetry/poetry_config.shand updatesbuild-poetrytestsBehavioral changes
build-poetryno longer fetches Artifactory reader credentials itself; that is handled byconfig-poetryconfig-poetrycan now be used standalone in workflows that need Poetry + Repox without a full buildPOETRY_HTTP_BASIC_REPOX_USERNAMEandPOETRY_HTTP_BASIC_REPOX_PASSWORDare exported viaGITHUB_ENVfor downstream stepsTest plan
shellspec spec/config-poetry_spec.sh spec/build-poetry_spec.shpasses locally