Copilot commented on code in PR #6824:
URL: https://github.com/apache/texera/pull/6824#discussion_r3636065035
##########
.github/workflows/codecov-upload.yml:
##########
@@ -55,66 +58,74 @@ on:
permissions:
contents: read
- actions: read # download-artifact needs this to read artifacts from another
run
+ actions: read # list + download artifacts from the source run
jobs:
- upload:
- # Skip only a cancelled/skipped upstream; still run on failure so partial
- # coverage is reported (mirrors the build's always()/!cancelled() staging).
+ discover:
+ # Which codecov-<flag> artifacts did the source run actually build? A
label-gated
+ # PR only stages some of them, so we list what exists and upload exactly
those —
+ # rather than a hardcoded flag set that throws "artifact not found" for
the rest.
+ # Skip only a cancelled/skipped upstream; still run on failure so partial
coverage
+ # is reported (mirrors the build's always()/!cancelled() staging).
if: >-
github.event_name == 'workflow_dispatch' ||
(github.event.workflow_run.conclusion != 'cancelled' &&
github.event.workflow_run.conclusion != 'skipped')
runs-on: ubuntu-latest
- # Job-level so the group can key on matrix.flag: one upload pass per source
- # run + flag, and a re-run of the source supersedes the previous attempt.
+ outputs:
+ flags: ${{ steps.list.outputs.flags }}
+ steps:
+ - name: List codecov-* artifacts in the source run
+ id: list
+ uses: actions/github-script@v9
+ env:
+ # Passed via env (not interpolated into the script body) so a
+ # workflow_dispatch-supplied run_id can't inject into the JS.
+ SOURCE_RUN_ID: ${{ github.event.workflow_run.id || inputs.run_id }}
+ with:
+ script: |
+ const runId = Number(process.env.SOURCE_RUN_ID);
+ const arts = await
github.paginate(github.rest.actions.listWorkflowRunArtifacts, {
+ owner: context.repo.owner, repo: context.repo.repo, run_id:
runId, per_page: 100,
+ });
Review Comment:
`workflow_dispatch` requires `run_id`, but it’s typed as a string and the
script currently does `Number(process.env.SOURCE_RUN_ID)` without validation. A
non-numeric value becomes `NaN` (or `0` for empty), which will cause a
confusing API failure. Validate the run id early and fail with a clear message.
##########
.github/workflows/codecov-upload.yml:
##########
@@ -55,66 +58,74 @@ on:
permissions:
contents: read
- actions: read # download-artifact needs this to read artifacts from another
run
+ actions: read # list + download artifacts from the source run
jobs:
- upload:
- # Skip only a cancelled/skipped upstream; still run on failure so partial
- # coverage is reported (mirrors the build's always()/!cancelled() staging).
+ discover:
+ # Which codecov-<flag> artifacts did the source run actually build? A
label-gated
+ # PR only stages some of them, so we list what exists and upload exactly
those —
+ # rather than a hardcoded flag set that throws "artifact not found" for
the rest.
+ # Skip only a cancelled/skipped upstream; still run on failure so partial
coverage
+ # is reported (mirrors the build's always()/!cancelled() staging).
if: >-
github.event_name == 'workflow_dispatch' ||
(github.event.workflow_run.conclusion != 'cancelled' &&
github.event.workflow_run.conclusion != 'skipped')
runs-on: ubuntu-latest
- # Job-level so the group can key on matrix.flag: one upload pass per source
- # run + flag, and a re-run of the source supersedes the previous attempt.
+ outputs:
+ flags: ${{ steps.list.outputs.flags }}
+ steps:
+ - name: List codecov-* artifacts in the source run
+ id: list
+ uses: actions/github-script@v9
+ env:
+ # Passed via env (not interpolated into the script body) so a
+ # workflow_dispatch-supplied run_id can't inject into the JS.
+ SOURCE_RUN_ID: ${{ github.event.workflow_run.id || inputs.run_id }}
+ with:
+ script: |
+ const runId = Number(process.env.SOURCE_RUN_ID);
+ const arts = await
github.paginate(github.rest.actions.listWorkflowRunArtifacts, {
+ owner: context.repo.owner, repo: context.repo.repo, run_id:
runId, per_page: 100,
+ });
+ const flags = [...new Set(
+ arts.map(a => a.name)
+ .filter(n => n.startsWith('codecov-'))
+ .map(n => n.slice('codecov-'.length))
+ )].sort();
+ core.info(`Source run ${runId} staged codecov flags:
${flags.join(', ') || '(none)'}`);
+ core.setOutput('flags', JSON.stringify(flags));
+
+ upload:
+ needs: discover
+ # Nothing to do when the source produced no codecov-* artifacts (also
avoids an
+ # empty matrix, which is an error).
+ if: ${{ needs.discover.outputs.flags != '' && needs.discover.outputs.flags
!= '[]' }}
+ runs-on: ubuntu-latest
+ # Job-level so the group can key on matrix.flag: one upload pass per
source run +
+ # flag, and a re-run of the source supersedes the previous attempt.
concurrency:
group: codecov-upload-${{ github.event.workflow_run.id || inputs.run_id
}}-${{ matrix.flag }}
cancel-in-progress: false
strategy:
fail-fast: false
matrix:
- # flag = artifact suffix (codecov-<flag>); coverage=false for the
- # test-results-only flag (amber-integration has no coverage report).
- #
- # KEEP IN SYNC with the coverage-producing jobs in build.yml: one row
per
- # codecov-<flag> artifact staged there — frontend, amber,
amber-integration,
- # pyamber, agent-service, and every service in build.yml's `platform`
matrix.
- # A flag staged in build.yml but missing here is downloaded by nothing
and
- # its coverage is silently dropped (the reverse — a row here with no
artifact
- # — is safe: the download continue-on-errors and the leg skips).
- include:
- - { flag: frontend, coverage: true }
- - { flag: amber, coverage: true }
- - { flag: amber-integration, coverage: false }
- - { flag: pyamber, coverage: true }
- - { flag: agent-service, coverage: true }
- - { flag: config-service, coverage: true }
- - { flag: access-control-service, coverage: true }
- - { flag: file-service, coverage: true }
- - { flag: computing-unit-managing-service, coverage: true }
- - { flag: workflow-compiling-service, coverage: true }
- - { flag: notebook-migration-service, coverage: true }
+ flag: ${{ fromJSON(needs.discover.outputs.flags) }}
Review Comment:
`fromJSON(needs.discover.outputs.flags)` can throw if
`needs.discover.outputs.flags` is an empty string (e.g., if `discover` is
skipped or fails before setting outputs). Providing a safe default avoids the
whole workflow failing during matrix evaluation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
