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

Upload results.json using raw api, rather than test-collector #433

Merged
merged 11 commits into from
Mar 11, 2025

Conversation

IanButterworth
Copy link
Member

Redo of #428
Incorporates #430

It's too difficult to use test-collector to upload these files with a custom job-id, and #428 did it in an unsafe way.

if compgen -G "${JULIA_INSTALL_DIR}/share/julia/test/results*.json"; then
(cd "${JULIA_INSTALL_DIR}/share/julia/test"; buildkite-agent artifact upload results*.json)
(cd "${JULIA_INSTALL_DIR}/share/julia/test"; buildkite-agent artifact upload "results*.json")
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes this from JuliaLang/julia#57686

2025-03-09 20:18:09 INFO   Found 1 files that match "results_ArgTools_1.json"
fatal: failed to upload artifacts: uploading artifacts: creating uploader: invalid upload destination: 'results_Artifacts_1.json'. Only s3://*, gs://*, rt://*, or https://*.blob.core.windows.net destinations are allowed. Did you forget to surround your artifact upload pattern in double quotes?

@IanButterworth IanButterworth marked this pull request as ready for review March 10, 2025 18:55
@IanButterworth
Copy link
Member Author

IanButterworth commented Mar 11, 2025

@staticfloat is this ok? One regression is the plugin did parallel uploads, which isn't an issue on julia master because we only have one results file, but with JuliaLang/julia#57686 we will have many.

I was thinking of merging this and testing it on that PR to see if I need to add the parallel uploads back?

@vchuravy
Copy link
Member

I don't think we need parallel uploads here.

@vchuravy
Copy link
Member

The only thing would be to open an issue with the test collector plugin and ask for support of a custom job_id

@IanButterworth
Copy link
Member Author

IanButterworth commented Mar 11, 2025

The only thing would be to open an issue with the test collector plugin and ask for support of a custom job_id

Already open buildkite-plugins/test-collector-buildkite-plugin#83
And a PR buildkite-plugins/test-collector-buildkite-plugin#84 but the PR doesn't work because the env in the pre-exit process doesn't see new env vars from the previous steps (and sourcing the env var file proved a bad idea because of buildkite/agent#3227)

@IanButterworth IanButterworth merged commit b5c7ac7 into JuliaCI:main Mar 11, 2025
6 checks passed
@IanButterworth IanButterworth deleted the ib/redo_test_job_id branch March 11, 2025 16:15
@staticfloat
Copy link
Member

Belated LGTM!

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.

3 participants