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

Download actions job logs from API #33858

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

lunny
Copy link
Member

@lunny lunny commented Mar 11, 2025

Related to #33709, #31416

It's similar with https://docs.github.com/en/rest/actions/workflow-jobs?apiVersion=2022-11-28#download-job-logs-for-a-workflow-run--code-samples.

This use job_id as path parameter which is consistent with Github's APIs.

@lunny lunny added type/enhancement An improvement of existing functionality modifies/api This PR adds API routes or modifies them labels Mar 11, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 11, 2025
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 11, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 11, 2025
@lunny lunny added this to the 1.24.0 milestone Mar 11, 2025
@wxiaoguang
Copy link
Contributor

I am sure you just copied & pasted the code from somewhere without understanding what each line does.

@ChristopherHX
Copy link
Contributor

ChristopherHX commented Mar 16, 2025

Works for me.

While testing I remembered workflow_job webhook uses jobid instead of job index for api url, to have the same field to url mapping for the url field as GitHub Rest.

e.g. http://localhost:3000/api/v1/repos/test/private-repo/actions/runs/842/jobs/373 (prefix of this logs endpoint)

instead of http://localhost:3000/api/v1/repos/test/private-repo/actions/runs/842/jobs/0.

And logs is below this path.

The swagger documention tells me I can use the id that is 404.

image

This use run_id as path parameter which is consistent with other APIs but not web routers. Maybe all web routers should change from run_index to run_id to keep consistent.

I would prefer run_id and job_id in both web routers and api. However I know that this is going to break existing commit status reports

I guess workflow_job webhook is currently a good way to obtain all ids to construct this url, we might need a workflow_run/workflow_job api route to make this really usable from scripts.

EDIT

Sorry some parts are incorrect from my side...

The gh compatible api path of workflow_job is the following, in this case only a id makes sense.

http://localhost:3000/api/v1/repos/test/private-repo/actions/jobs/373

@ChristopherHX
Copy link
Contributor

We were looking at the wrong section of the gh docu.

You are implementing this endpoint https://docs.github.com/en/rest/actions/workflow-jobs?apiVersion=2022-11-28#download-job-logs-for-a-workflow-run--code-samples under a different url.

The one from GitHub has no runs part in the url and uses the jobid instead of index

@lunny
Copy link
Member Author

lunny commented Mar 16, 2025

job_id is a string which is from the workflow not the id stored in the database.
ref https://docs.github.com/en/enterprise-cloud@latest/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_id

@ChristopherHX
Copy link
Contributor

ChristopherHX commented Mar 16, 2025

job_id is a string which is from the workflow not the id stored in the database.

GitHub is good in mixing names, yes job_id in workflow yaml is a string.

job_id in workflow_job api object is a number from the database see here:

ref https://docs.github.com/en/rest/actions/workflow-jobs?apiVersion=2022-11-28#get-a-job-for-a-workflow-run

The example has a really big number, that is a global database id I guess

"id": 399444496,

GitHub Actions users have hard time to correlate a workflow execution from the runner to a workflow_job api id

@lunny lunny changed the title Download actions logs from API Download actions job logs from API Mar 17, 2025
Copy link
Contributor

@ChristopherHX ChristopherHX 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, left some non functional suggestions

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants