Repository navigation
gh run rerun --job is not working as documented #8967
Description
Activity
Okeakwalam27 commented
on Apr 17, 2024 on Apr 17, 2024 via email · Hidden as spamshow commentMore actionsReading the code for the rerun & the beginning of the doc string, I think the intention really is that you either provide a job ID or the run ID. Maybe the issue here is more about the documentation for that subcommand which:
- makes it seem like you can provide
--joband runid at the same time - parameter is string type, not a number so that makes the confusion worse
- makes it seem like you can provide
Yes I can see why this would be confusing. I agree with what you said about the docs, and I think there's a couple of other things that could be improved. In summary:
- Because the command was originally created with a positional arg for the
run IDit seems like it can be provided with the job ID to narrow it down. - Because the field is
--joband not--job-idit looks like you might be expected to provide the name - Because the field is a string type and not a number, it looks like you might be expected to provide the name
- Because the docs list the
{ name, databaseId }it looks like you might be expected to provide the name
So what can we do?
1. Positional vs Flag Args
In terms of the command, we can't really change much about this for backwards compatibility reasons. We need to continue supporting both the positional arg and the flag. The endpoints that are hit in providing these IDs are completely different:
path := fmt.Sprintf("repos/%s/actions/runs/%d/%s", ghrepo.FullName(repo), run.ID, runVerb)and
path := fmt.Sprintf("repos/%s/actions/jobs/%d/rerun", ghrepo.FullName(repo), job.ID)We could allow the
runIDto be provided with thejobIDand then at this pointcheck that the providedcli/pkg/cmd/run/rerun/rerun.go
Lines 107 to 114 in 8009e79
if jobID != "" { opts.IO.StartProgressIndicator() selectedJob, err = shared.GetJob(client, repo, jobID) opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to get job: %w", err) } runID = fmt.Sprintf("%d", selectedJob.RunID) runIDmatches the one in the--jobflag.' This error would say something like:The job with ID <JOB_ID> does not belong to a run with ID <RUN_ID> but <ACTUAL_RUN_ID>. Note that if the `--job` flag is provided, it is not necessary to provide a run ID.Alternatively, it looks like
gh run viewprints a warning that it is ignoring therun IDLines 164 to 170 in bdff1f1
if opts.RunID != "" && opts.JobID != "" { opts.RunID = "" if opts.IO.CanPrompt() { cs := opts.IO.ColorScheme() fmt.Fprintf(opts.IO.ErrOut, "%s both run and job IDs specified; ignoring run ID\n", cs.WarningIcon()) } } 2. Flag naming
Again due to backwards compatibility we can't just change the name of this flag to
--job-id. We could deprecate it, but I think perhaps just being clearer in the job description that this is expected to be an ID would be enough.3. String type
I think we could change this to an
int64. ThedatabaseIDthat is used should always parse into anint64and I don't believe it should ever be a string.4. { name, databaseId }
I wrote this doc comment after using the wrong ID so many times. I included the
nameso that people running the command would be able to match the ID to the correct job. I think the comment is good, we probably just need to call out that it's thedatabaseIDthat should be selected.
What do you think about these? Any further thoughts?
- Because the command was originally created with a positional arg for the
- addedmore-info-neededMore info needed from user/contributorMore info needed from user/contributorand removedneeds-triageneeds to be reviewedneeds to be reviewed
on Apr 17, 2024 @williammartin Generally agreed with your points. More specifically
- I value consistency so doing what
gh run viewdoes (print a warning) seems like a good option - I agree if the flag description is updated to include "ID" in some form it will be clear enough
- That will help strengthen point 2.
- Yup, making it clearer about databaseID might help too
When combined the proposed changes sound good enough. Certainly no need to deprecate stuff or break compat.
- I value consistency so doing what
Cool cool, are you interested in opening a PR to get these changes made?
- addedhelp wantedContributions welcomeContributions welcomegh-runrelating to the gh run commandrelating to the gh run commandand removedmore-info-neededMore info needed from user/contributorMore info needed from user/contributor
on Apr 17, 2024 Cool cool, are you interested in opening a PR to get these changes made?
Sure, the changes seem small enough. I'll have a look in the next few days
Reacted by William Martin
Describe the bug
This is what gh run rerun -h help looks like in the most recent version:
Specifically, one of the flags is
-j(or--job) to rerun a job with specific name. However:I haven't yet looked at the code but seems to me that arg parsing is wrong? Rerunning a job name without providing a run number does not make sense I believe.
Steps to reproduce the behavior
Expected vs actual behavior
I'd expect to be able to re-run a job with specific from a given run with
--jobswitchLogs
Paste the activity from your command line. Redact if needed.