comphead commented on code in PR #5782: URL: https://github.com/apache/datafusion-comet/pull/5782#discussion_r3970037841
########## .github/actions/upload-artifact-retry/action.yaml: ########## @@ -0,0 +1,139 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: "Upload Artifact (with retry)" +description: > + Drop-in replacement for actions/upload-artifact that retries the upload three + times. The artifact client only retries 429/500/502/503/504, so a + FinalizeArtifact answered "(403) Forbidden: Error from intermediary" fails the + step even though the content uploaded fine. There is no input to widen that + list, Actions has no built-in step retry, and third-party retry wrappers are + not on the Apache allowed-actions list. See .github/workflows/README.md. + +# Inputs mirror actions/upload-artifact@v7 one for one. The boolean defaults +# have to be real 'true'/'false' strings because core.getBooleanInput() throws +# on an empty value; the numeric ones default to '' so "unset" round-trips. +inputs: + name: + description: 'Artifact name' + required: false + default: 'artifact' + path: + description: 'A file, directory or wildcard pattern that describes what to upload' + required: true + if-no-files-found: + description: "Behavior if no files are found: warn, error or ignore" + required: false + default: 'warn' + retention-days: + description: 'Days before the artifact expires (empty means repository default)' + required: false + default: '' + compression-level: + description: 'Zlib compression level 0-9 (empty means the action default)' + required: false + default: '' + overwrite: + description: 'Delete an existing artifact with the same name before uploading' + required: false + default: 'false' + include-hidden-files: + description: 'Include hidden files in the artifact' + required: false + default: 'false' + archive: + description: 'Zip the content before uploading' + required: false + default: 'true' + +# `||` yields the first non-empty operand, so this picks whichever attempt ran. +outputs: + artifact-id: + description: 'ID of the uploaded artifact' + value: ${{ steps.attempt-1.outputs.artifact-id || steps.attempt-2.outputs.artifact-id || steps.attempt-3.outputs.artifact-id }} + artifact-url: + description: 'Download URL of the uploaded artifact' + value: ${{ steps.attempt-1.outputs.artifact-url || steps.attempt-2.outputs.artifact-url || steps.attempt-3.outputs.artifact-url }} + artifact-digest: + description: 'SHA-256 digest of the uploaded artifact' + value: ${{ steps.attempt-1.outputs.artifact-digest || steps.attempt-2.outputs.artifact-digest || steps.attempt-3.outputs.artifact-digest }} + +runs: + using: "composite" + steps: + # continue-on-error keeps a failed attempt from failing the job while still + # recording outcome == 'failure', which the later attempts gate on. The last + # attempt omits it so a genuinely broken upload still fails loudly. + - name: Upload ${{ inputs.name }} (attempt 1 of 3) + id: attempt-1 + uses: actions/upload-artifact@v7 + continue-on-error: true + with: + name: ${{ inputs.name }} + path: ${{ inputs.path }} + if-no-files-found: ${{ inputs.if-no-files-found }} + retention-days: ${{ inputs.retention-days }} + compression-level: ${{ inputs.compression-level }} + overwrite: ${{ inputs.overwrite }} + include-hidden-files: ${{ inputs.include-hidden-files }} + archive: ${{ inputs.archive }} + + - name: Wait before retrying ${{ inputs.name }} + if: ${{ steps.attempt-1.outcome == 'failure' }} + shell: bash + run: | + echo "::warning::Upload of '${{ inputs.name }}' failed; retrying in 15s (attempt 2 of 3)." + sleep 15 + + # Retries force overwrite: attempt 1 may have created the server-side record + # before failing, and CreateArtifact rejects a duplicate name. The delete is + # best effort inside upload-artifact, so it no-ops when nothing exists. + - name: Upload ${{ inputs.name }} (attempt 2 of 3) + id: attempt-2 + if: ${{ steps.attempt-1.outcome == 'failure' }} + uses: actions/upload-artifact@v7 + continue-on-error: true + with: + name: ${{ inputs.name }} + path: ${{ inputs.path }} + if-no-files-found: ${{ inputs.if-no-files-found }} + retention-days: ${{ inputs.retention-days }} + compression-level: ${{ inputs.compression-level }} + overwrite: 'true' Review Comment: Fixed in 5db3fc7. Each producer now qualifies the name with its version inputs, and the consumers follow: - `spark_sql_test_reusable.yml`: `native-lib-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}`, matching the `jvm-compiled-spark-<full>-jdk<N>` the same job already publishes. - `iceberg_spark_test_reusable.yml`: `native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}`. `native-lib-linux` in `pr_build_linux.yml` and `native-lib-macos` in `pr_build_macos.yml` keep their bare names: `ci.yml` calls each of those workflows exactly once, so after the rename each has a single producer in the run. Since a shared name is invisible until a retry lands on it, the invariant is now enforced rather than just fixed. `dev/ci/check-ci-config.py` (new, run from `preflight`) fails when a reusable workflow that `ci.yml` calls more than once uploads an artifact whose name carries no `inputs.` reference, and when a `download-artifact` name is not produced by an upload in the same workflow. I checked it by fault injection: reverting just the Spark producer back to `native-lib-linux` reports both the shared name and the now-orphaned consumer. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
