Skip to content

Conversation

@doanac
Copy link
Contributor

@doanac doanac commented May 14, 2025

No description provided.

@ricardosalveti ricardosalveti requested a review from lool May 14, 2025 21:14
Copy link

@ricardosalveti ricardosalveti left a comment

Choose a reason for hiding this comment

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

Nice

Copy link
Contributor

@lool lool left a comment

Choose a reason for hiding this comment

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

I guess this is a draft PR as I see some DEBUG entries in it still?

with:
path: /tmp/${BUILD_ID}
build_id: "${{ github.repository }}-${{ github.run_id }}-${{ github.run_attempt }}"
github_token: "${{ secrets.GITHUB_TOKEN }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised you need to pass this specifically in the yaml; isn't GITHUB_TOKEN provided to actions by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its a security thing. GitHub doesn't want you to accidentally share the token with a workflow you haven't explicitly granted permissions to.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is is possible to implement an action like the github upload-artifact action, without passing the token? Does that require some lengthy certification process with github or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've come up with a better way. Its slight complicated to get going but will actually work much better and get us working just like the public action. I'll redo this PR once I get it working

uses: qualcomm-linux/upload-private-artifact-action@v1
with:
path: /tmp/${BUILD_ID}
build_id: "${{ github.repository }}-${{ github.run_id }}-${{ github.run_attempt }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is more some thoughts on the API of this action, but I wonder if we should keep the concept of a build_id local to the repositories that needs one.

As an user of such an action, I would expect it to provide similar behavior as actions/upload-artifact. In particular, I expect that the artifacts uploaded from one repo are stored in a repo specific collection of artifacts.

When using upload-artifact, most parameters are optional. Perhaps our upload-private-artifact could provide similar behavior and there would be a download action to easily retrieve a particular artifact or the latest artifact with some name (like we're doing in the debos recipe to fetch u-boot/linux builds prior to building an image).

- /efs/qli/metaqcom/gh-runners/quic-yocto/downloads:/fileserver-downloads
options: --privileged
steps:
- name: TMP-DEBUG
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this is going away in the final version?

@doanac doanac changed the title ci: Move to new artifact upload action WIP: ci: Move to new artifact upload action May 15, 2025
Copy link
Contributor

@lool lool left a comment

Choose a reason for hiding this comment

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

I think this is really looking really nice now, from the perspective of this repository!

@doanac doanac force-pushed the upload-action branch 6 times, most recently from 385c429 to 19e83f1 Compare May 16, 2025 15:46
@doanac doanac changed the title WIP: ci: Move to new artifact upload action ci: Move to new artifact upload action May 16, 2025
@doanac
Copy link
Contributor Author

doanac commented May 16, 2025

Okay - i've cleaned up the action and backend code in a way to make this work fairly cleanly

@lool
Copy link
Contributor

lool commented May 18, 2025

I liked all the removals in the latest version :)

@doanac
Copy link
Contributor Author

doanac commented May 19, 2025

I think this is ready if you are satisfied. It turned into way more backend work than I was ever wanting to do for this but it should make the transition work for other repos go more smoothly as we get them on the new runners.

@lool lool merged commit 4b15a29 into qualcomm-linux:main May 19, 2025
5 checks passed
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