Skip to content

refactor: replace boto3-based upload with AWS CLI#10

Merged
doanac merged 1 commit intoqualcomm-linux:awsfrom
quic-viskuma:aws
Sep 23, 2025
Merged

refactor: replace boto3-based upload with AWS CLI#10
doanac merged 1 commit intoqualcomm-linux:awsfrom
quic-viskuma:aws

Conversation

@quic-viskuma
Copy link

@quic-viskuma quic-viskuma commented Sep 19, 2025

Simplifies the artifact upload process by removing Python dependencies and switching to AWS CLI.

  • Deleted publish_artifacts.py and requirements.txt
  • Updated action.yml to use aws s3 cp for uploading artifacts
  • Ensures compatibility with both Ubuntu and Debian environments

Fixes: #9

Copy link

@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.

Looks good to me, thanks!

I see the unit tests ran, but against the main version of the action which uploads to GCP; I'll file a separate issue for that

@lool
Copy link

lool commented Sep 19, 2025

@quic-viskuma If you add Fixes: #9 to your commit, it will automatically close the issue when your commit gets merged

Simplifies the artifact upload process by removing Python dependencies and switching to AWS CLI.
- Deleted publish_artifacts.py and requirements.txt
- Updated action.yml to use aws s3 cp for uploading artifacts
- Ensures compatibility with both Ubuntu and Debian environments

Fixes: qualcomm-linux#9

Signed-off-by: Vishal Kumar <viskuma@qti.qualcomm.com>
Copy link
Contributor

@doanac doanac left a comment

Choose a reason for hiding this comment

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

looks good. this requires the workflow to run as root when aws-cli isn't installed. I think this is the case for our workflows, right?

@lool
Copy link

lool commented Sep 23, 2025

looks good. this requires the workflow to run as root when aws-cli isn't installed. I think this is the case for our workflows, right?

I think we run as non-root on the runners, and as root in the containers; our images seem to have aws pre-installed. So this code should run in both cases (and I believe was tested for both cases).

One possible improvement would be to use sudo to apt install when running as non-root, but I guess it's not really a situation we're expecting to be in for this qualcomm specific action, so perhaps we can defer until we hit it? Same for non-deb/apt based OSes.

@quic-viskuma
Copy link
Author

looks good. this requires the workflow to run as root when aws-cli isn't installed. I think this is the case for our workflows, right?

I think we run as non-root on the runners, and as root in the containers; our images seem to have aws pre-installed. So this code should run in both cases (and I believe was tested for both cases).

One possible improvement would be to use sudo to apt install when running as non-root, but I guess it's not really a situation we're expecting to be in for this qualcomm specific action, so perhaps we can defer until we hit it? Same for non-deb/apt based OSes.

Yes, I have tested with AWS runners which we're using in our workflows and also with containers. I'm not sure if we should add sudo while installing, I can test this. We also need to consider the other OSes- like fedora in future.

@lool
Copy link

lool commented Sep 23, 2025

Let's land this now and add support for Fedora when that comes?

@doanac doanac merged commit 4ca3f68 into qualcomm-linux:aws Sep 23, 2025
3 checks passed
@doanac
Copy link
Contributor

doanac commented Sep 23, 2025

I've merged this and tagged it as aws-v2

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.

4 participants