ci: Convert to a newer publishing API #42
Conversation
|
NOTE: This script is a stop-gap measure while we wait for approval to make a common github action for our QLI repos to use. |
lool
left a comment
lool
left a comment
There was a problem hiding this comment.
Overall looks good to me, I've left a few suggestions for improvements if you have the chance
| # python3-requests is used by publish_aritfacts.py | ||
| apt -y install python3-requests | ||
| # create a directory for the current run | ||
| dir="/fileserver-builds/${BUILD_ID}" |
There was a problem hiding this comment.
I guess we can drop this volume now:
- /srv/gh-runners/quic-yocto/builds:/fileserver-builds
There was a problem hiding this comment.
good catch. should be addressed in force push.
| @@ -0,0 +1,104 @@ | |||
| #!/usr/bin/env python3 | |||
| # Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved. | |||
| # SPDX-License-Identifier: MIT | |||
There was a problem hiding this comment.
I guess the workflow files should have a license, the debos recipes and shell scripts use BSD-3-Clause and that's the license in the repo and usually what Qualcomm defaults to, so perhaps let's use BSD-3-Clause or we need to add MIT license to the repo as LICENSE.MIT or something.
There was a problem hiding this comment.
as it was copied directly from meta-qcom, I wasn't sure.
| if URL[-1] != "/": | ||
| URL = URL + "/" | ||
|
|
||
| main(BUILD_DIR, URL) |
There was a problem hiding this comment.
I guess this script will be replaced soon, but the BUILD_DIR/URL env inputs are not easy to guess when reading the workflow files. Perhaps these should be passed as args publish-artifacts.py some-dir some-url or public-artifacts.py --build-dir dir --url url
There was a problem hiding this comment.
I prefer your approach as well, but I'd like to keep this the exact same as meta-qcom for now and then move things over to the github action once approved (should be soon i think).
|
|
||
| failed = False | ||
| work = [(f"{base_url}{x}", artifacts_dir, x) for x in paths] | ||
| with Pool(5) as p: |
There was a problem hiding this comment.
maybe move the 5 to a constant at the top
There was a problem hiding this comment.
addressed in force-push
|
|
||
| # Fibonacci backoff | ||
| for x in (1, 2, 3, 5, 0): | ||
| r = requests.put(url, headers=headers, allow_redirects=False) |
There was a problem hiding this comment.
I don't understand why we need two put requests; IIUC, we get redirected by the web service to the actual place where we're supposed to upload? Perhaps this scheme should be capture in a comment in the file?
There was a problem hiding this comment.
i've added a docstring in the force-push
This comes from meta-qcom. Its a temporary thing while we wait for permission to create a common Github Action. Signed-off-by: Andy Doan <[email protected]>
By publishing this way, we are no longer dependent on NFS file share. This will make it easier to transition to the new IT provided self-hosted runners. Signed-off-by: Andy Doan <[email protected]>
Signed-off-by: Andy Doan <[email protected]>
Signed-off-by: Andy Doan <[email protected]>
|
Seems fine overall, there were some HTTP 500 in this run, which went away after retry, I don't know if that's common, I see you've implemented backoff/retry logic originally in the script, so I guess that's something that's happening regularly. This makes me think we should probably avoid hitting github or our GCP server with a bunch of requests, but rather a single one to upload multiple assets, this would simplify the logic dramatically. I believe that's what github upload-artifacts does – it creates an archive and uploads that. |
2 participants
The current artifact upload logic requires the self-hosted runner and the file-server to share the same NFS volume as a means of uploading artifacts. This change decouples that so that we could use any runner and still upload artifacts.
We do this by leveraging the GITHUB_TOKEN included in every CI job (including PRs from forked repositories). This is an ephemeral token only valid during the lifetime of the workflow. By granting it read access to the "security-events" API, our file-serve can assert the token it gets is a valid qualcomm-linux token before allowing an upload to happen.
The upload logic gets a little hard to maintain with inline yaml and shell script, so I've created a Python helper to do the upload. It has a few handy features: