Allows the user to pass in a commit SHA to use instead of relying on the payload or head commit. The main use-case of this is for workflow dispatch runs where the SHA is provided through an input.

This change should be valid based onhttps://docs.github.com/en/rest/reference/commits#get-a-commit.

jefchien force-pushed theadd-optional-commit-sha branch from4ab55a4 to35c0db2Compare

December 17, 2021 21:25

Add optional commit SHA in config.

dd836ac

jefchien force-pushed theadd-optional-commit-sha branch from35c0db2 todd836acCompare

December 18, 2021 00:16

jefchien mentioned this pull request

Dec 20, 2021

Trigger performance test after successful CI run.aws-observability/aws-otel-collector#818

Merged

khanhntd reviewed

Dec 21, 2021

View reviewed changes

src/extract.ts


		asyncfunctiongetCommit(githubToken?:string):Promise<Commit>{
		asyncfunctiongetCommit(githubToken?:string,commitSha?:string):Promise<Commit>{
		if(commitSha&&githubToken){

Copy link

Choose a reason for hiding this comment

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

Should this line be placed after the line253-261 ? Mostly I can't see any reason to let this success faster though.

Copy link

Author

Choose a reason for hiding this comment

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

Well, if you've provided the commit SHA, then your intention is for the action to use it. I don't see why it should even attempt to use the other options.

khanhntd approved these changes

Dec 21, 2021

View reviewed changes

Copy link

khanhntd left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link

Member

Thank you for your contribution! Could you please explain to me a bit more why would we want to provide the SHA as an input rather than just using the one provided via pull request or fall back to the current commit's sha?

Copy link

Author

jefchien commentedDec 21, 2021

Hi@ktrz,

So, in my use case, the workflow where the action is used can be triggered by a workflow dispatch where the SHA is provided as an input. This allows us to run the benchmarking after successful CI workflow runs or on selected commits. The issue I've found is that thegithub.context.ref used in the fall back is the workflow commit SHA rather than the one actually used in the benchmarking. If there's another way to access the workflow dispatch inputs, it might be a better solution. This PR provides an optional field that will supersede any of the other attempts.

Copy link

Contributor

NathanielRN commentedDec 22, 2021

@jefchien

Is running theworkflow_dispatch job event an automated process? Instead of providing thecommit_sha as input, would you be able to push a branch or tag and run the workflow from there instead?

That way you can make the branch point to the commit you want, and run the workflow from that branch, so that the commit is correct and you don't need to provide thecommit_sha as input.

It might be more tricky if your invocation of theworkflow_dispatch job is automated (because you'd need a GitHub account to publish it from I guess) so that's why I ask.

Copy link

Author

jefchien commentedJan 6, 2022

@NathanielRN

Running theworkflow_dispatch is a manual process, but the same workflow is also triggered by arepository_dispatch from a different workflow. That's an interesting workaround. I suppose I could use a GitHub action likehttps://github.com/marketplace/actions/create-branch to create the branch as part of the workflow and then checkout that branch in the job used to run this action. Actually, that probably won't work since the GitHub context is based on the workflow run.