sunchao commented on code in PR #5782: URL: https://github.com/apache/datafusion-comet/pull/5782#discussion_r3964475838
########## .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: ### Correctness [P2] Give parallel producers distinct artifact names before retrying with overwrite `ci.yml` runs the Linux native build and the Spark 3.5/4.1 builds in parallel, and all three upload `native-lib-linux`. [The v7 delete selector](https://github.com/actions/upload-artifact/blob/043fb46d1a93c77aae656e7c1c64a875d1fc6a0a/dist/upload/index.js#L129059-L129090) selects the newest artifact with that name and sends its producer's job ID to `DeleteArtifact`. The [pinned action documentation](https://github.com/actions/upload-artifact/blob/043fb46d1a93c77aae656e7c1c64a875d1fc6a0a/README.md#overwriting-an-artifact) also explicitly supports overwriting an artifact from a different job. If one producer finishes while another fails before creating its artifact, this retry can delete the completed producer's artifact. Consumers depend only on their own producer and can then fail lookup during the replacement upload, or if that upload also fails. Please make the producer and download names unique per workflow/version before enabling forced overwrite. The parallel Iceberg versions also share `native-lib-iceberg` when those jobs are enabled. -- 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]
