Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade actions/upload-artifact to match actions/download-artifact #5428

Merged
merged 4 commits into from
Sep 16, 2024

Conversation

rdettai
Copy link
Collaborator

@rdettai rdettai commented Sep 16, 2024

Description

Looks like the bump from actions/download-artifact 3 to 4 broke the docker CI (https://github.com/quickwit-oss/quickwit/actions/workflows/publish_docker_images.yml). This is an expected breaking change (https://github.com/actions/download-artifact?tab=readme-ov-file#breaking-changes))

How was this PR tested?

Describe how you tested this PR.

Copy link
Collaborator Author

@rdettai rdettai left a comment

Choose a reason for hiding this comment

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

In the breaking change section of this new version there is the following item:

Uploading to the same named Artifact multiple times. Due to how Artifacts are created in this new version, it is no longer possible to upload to the same named Artifact multiple times. You must either split the uploads into multiple Artifacts with different names, or only upload once. Otherwise you will encounter an error.

I think this will likely be a problem as we repeatedly use the same artifact names. But it's not completely clear from the docs whether this restriction applies only within a job run or in general.

@trinity-1686a
Copy link
Contributor

https://github.com/quickwit-oss/quickwit/actions/runs/10879677075

@trinity-1686a
Copy link
Contributor

Run actions/[email protected]
With the provided path, there will be 1 file uploaded
Artifact name is valid!
Root directory input is valid!
Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run

Copy link

github-actions bot commented Sep 16, 2024

On SSD:

Average search latency is 0.994x that of the reference (lower is better).
Ref run id: 3431, ref commit: 2d4f5a6
Link

On GCS:

Average search latency is 0.996x that of the reference (lower is better).
Ref run id: 3432, ref commit: 2d4f5a6
Link

@rdettai rdettai force-pushed the artifact-action-upgrade branch from e90e9cf to 6ed8059 Compare September 16, 2024 09:44
@rdettai
Copy link
Collaborator Author

rdettai commented Sep 16, 2024

@rdettai
Copy link
Collaborator Author

rdettai commented Sep 16, 2024

Copy link
Contributor

@trinity-1686a trinity-1686a left a comment

Choose a reason for hiding this comment

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

ERROR: can't push with no tags specified, please set --tag or --dry-run

looks like the kind of error we'd expect from running that way, we can ignore it

@rdettai
Copy link
Collaborator Author

rdettai commented Sep 16, 2024

looks like the kind of error we'd expect from running that way, we can ignore it

Yes, I was reaching the same conclusion! Let's see what comes out of a merge! (built a small repo https://github.com/rdettai/testartifact to double check upload/download with both versions are now equivalent)

@rdettai rdettai force-pushed the artifact-action-upgrade branch from 202b98d to 706f94a Compare September 16, 2024 13:03
@rdettai rdettai enabled auto-merge (squash) September 16, 2024 13:03
@rdettai rdettai merged commit 740b2ba into main Sep 16, 2024
5 checks passed
@rdettai rdettai deleted the artifact-action-upgrade branch September 16, 2024 13:04
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