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

Rework getWorkflowRunId() when run-name is provided #17

Merged
merged 11 commits into from
Jun 19, 2024

Conversation

LudovicTOURMAN
Copy link

@LudovicTOURMAN LudovicTOURMAN commented Dec 29, 2023

Issue:
As discussed here, I figured out that the current function octokit.rest.checks.listForRef, which is based on repos/OWNER/REPO/commits/REF/check-runs, and used to retrieve the run id, when a run-name is provided, is not working.

Cause is that check_name is filled by Github with the jobs.<job_id> (or jobs.<job_id>.name) and not the run-name we are expecting.

Solution:
The default octokit.rest.actions.listWorkflowRuns currently used is also the proper one to filter based on the name.
So once we use the method, we only need to do an extra filter on the name attribute to match the proper run-name.
To limit the number of results, created parameter has also been set on the API call.

Since the two recent functions findWorkflowRunIdFromFirstRunOfSameWorkflowId and findWorklowRunIdFromRunName are merged back into getWorkflowRunId, I cleaned them.

Extra changes:

  • Add workflow-id as output
  • Add 2 new modes for workflow-logs:
    • output: Set remote workflow logs (as raw) in workflow-logs output
      <job-name> | yyyy-MM-ddTHH:mm:ss.SSSSSSSZ <message>
      <job-name> | yyyy-MM-ddTHH:mm:ss.SSSSSSSZ <message>
      
    • json-output: Set remote workflow logs (as json object) in workflow-logs output
      {
        "<job-name>": [
          {
            "datetime": "yyyy-MM-ddTHH:mm:ss.SSSSSSSZ",
            "message": "<message>"
          },
          {
            "datetime": "yyyy-MM-ddTHH:mm:ss.SSSSSSSZ",
            "message": "<message>"
          }
        ]
      }
  • README.md
    • Do some lint
    • Add an extra example (with corresponding remote workflow) when using run-name
  • action.yaml
    • Add outputs definition
  • Fix some typos (shoud, tirggered, Muse)

@LudovicTOURMAN
Copy link
Author

Here are the test results for the last commit 1fda9fc: https://github.com/LudovicTOURMAN/workflow-dispatch/actions/runs/7395335486

Copy link
Member

@evanlindsey evanlindsey left a comment

Choose a reason for hiding this comment

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

lgtm!

@LudovicTOURMAN
Copy link
Author

Hello @aurelien-baudet, will you be able to have a look at this PR? Thank you 😊

@evanlindsey
Copy link
Member

Hello @aurelien-baudet, will you be able to have a look at this PR? Thank you 😊

bump

@thepsc-go
Copy link

thepsc-go commented Mar 6, 2024

@LudovicTOURMAN Hi, looks like this action is abandoned from the Author. You seem like to know the topic as above discussion takes place. Can you fork the original one 'benc-uk/workflow-dispatch' (which looks also abandoned) towards your repo GH actions?
and upgrade it to Node20 ?

@ALEEF02
Copy link

ALEEF02 commented Mar 27, 2024

@LudovicTOURMAN Bump.

@LudovicTOURMAN Hi, looks like this action is abandoned from the Author. You seem like to know the topic as above discussion takes place. Can you fork the original one 'benc-uk/workflow-dispatch' (which looks also abandoned) towards your repo GH actions? and upgrade it to Node20 ?

Copy link
Collaborator

@aurelien-baudet aurelien-baudet left a comment

Choose a reason for hiding this comment

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

Please make some small changes to make code more readable.
I will merge once it's done.

src/workflow-handler.ts Show resolved Hide resolved
src/workflow-handler.ts Outdated Show resolved Hide resolved
@LudovicTOURMAN
Copy link
Author

Please make some small changes to make code more readable. I will merge once it's done.

Thanks for the feedback @aurelien-baudet. I've been able to rework the function to make it lighter and more readable.

@fbiesse
Copy link
Collaborator

fbiesse commented May 9, 2024

Please make some small changes to make code more readable. I will merge once it's done.

Bump

@aurelien-baudet
Copy link
Collaborator

Sorry, I have no more time and energy to maintain this repository. I even don't have computer anymore. Someone of you should for this repository and merge the different pull requests.
Tell me who creates the fork and I will archive this repository but indicate which repository follows up

@fbiesse
Copy link
Collaborator

fbiesse commented May 13, 2024

@LudovicTOURMAN I wanted to know if you're planning to take over on this action.
If not, I propose to create a fork, update readme, integrate your developments and maintain it.
Tell me

@LudovicTOURMAN
Copy link
Author

@LudovicTOURMAN I wanted to know if you're planning to take over on this action. If not, I propose to create a fork, update readme, integrate your developments and maintain it. Tell me

Hello @fbiesse, we are already using some private fork within my company. So I'm not going to maintain another public one. Go ahead if you would like to pick all of those for the community (and thanks).

@aurelien-baudet, no worry. That was still nice you provided this composite. Thanks for the work you've done and have a nice "repo-tirement"

@fbiesse
Copy link
Collaborator

fbiesse commented May 13, 2024

@LudovicTOURMAN Thank you, actually I have the same need for my company.
@aurelien-baudet Thank you too for this repo and all the work done on this one, I'll do my fork in the week.

@fbiesse
Copy link
Collaborator

fbiesse commented May 14, 2024

@aurelien-baudet I created the fork with all PR merged ?
Can you please transfer the action to my repo (https://github.com/fbiesse/workflow-dispatch)
https://docs.github.com/en/actions/creating-actions/publishing-actions-in-github-marketplace#transferring-an-action-repository
Thank you again for all the work.

@aurelien-baudet
Copy link
Collaborator

I tried to transfer repository but it fails with "fbiesse/workflow-dispatch already exists".
Maybe the best would be to create an organization instead of a single user to be able to maintain it by several people. So I could then transfer ownership to this organization and let you handle several maintainers and admins

@evanlindsey
Copy link
Member

@aurelien-baudet let’s do it. i just created @the-actions-org and invited you. we can add users from this thread and future maintainers once the repo is transferred.

@aurelien-baudet
Copy link
Collaborator

There is an error "You don’t have the permission to create public repositories on the-actions-org".
I think I must be admin of this organization first before being able to transfer the repository

@evanlindsey
Copy link
Member

There is an error "You don’t have the permission to create public repositories on the-actions-org". I think I must be admin of this organization first before being able to transfer the repository

It looks like the transfer succeeded. I went ahead and changed the base permissions for org members to write. Let me know if you’re still seeing issues?

@evanlindsey evanlindsey dismissed aurelien-baudet’s stale review May 29, 2024 16:58

changes applied by LudovicTOURMAN

@evanlindsey
Copy link
Member

evanlindsey commented May 29, 2024

@fbiesse I created the org and took transfer of the repo to help expedite remaining actions and responses (since I still receive updates from this thread). I'm not really interested in maintaining it, as I found a different solution and no longer use this. I added you to the-actions-org and made you a maintainer on this repo. @LudovicTOURMAN I gave you write permissions to this repo, which should allow you to go ahead and merge this PR.

@fbiesse
Copy link
Collaborator

fbiesse commented May 29, 2024

@evanlindsey thank you, it will be definitely more convenient. The think I can do is to create a PR with the last things I did on my fork (it already have both PR merged and tested) and rename the username part in the Readme. I'll be back of vacation in 2 weeks, I'll do it when I'll be back.

@fbiesse
Copy link
Collaborator

fbiesse commented Jun 11, 2024

@evanlindsey I'm back from vacations, but I couldn't accept the invitation to the new repo in time.
Can you please resend me the invitation ?
Thank you.

@evanlindsey
Copy link
Member

evanlindsey commented Jun 12, 2024

@evanlindsey I'm back from vacations, but I couldn't accept the invitation to the new repo in time. Can you please resend me the invitation ? Thank you.

@fbiesse welcome back and no problem! it looks like you were able to accept the member invite for the-actions-org and I just gave you the maintain role on this specific repo.

@fbiesse
Copy link
Collaborator

fbiesse commented Jun 12, 2024

@evanlindsey perfect, thank you !

@fbiesse fbiesse merged commit e845bf7 into the-actions-org:master Jun 19, 2024
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.

6 participants