FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: persist inputs between the upload action and its post step by NlightNFotis · Pull Request #2557 · github/codeql-action · GitHub

/ codeql-action Public

fix: persist inputs between the upload action and its post step - #2557

Merged
NlightNFotis merged 6 commits into
mainfrom
persist-inputs
Oct 21, 2024
ConversationCommits (6)ChecksFiles changed
Merged

fix: persist inputs between the upload action and its post step#2557
NlightNFotis merged 6 commits into
mainfrom
persist-inputs

Conversation

NlightNFotis commented Oct 21, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Description

This PR is fixing an issue with persisting action inputs between steps of the runner. This is an issue that is downstream of a problem with the GitHub Actions runner, ref: actions/runner#3514

Closes #2553

Merge / deployment checklist

Acknowledgments

  • Thanks to @chrisgavin for the code that formed the basis of this PR, and @jsoref for the original issue report, a PR containing code leading us to a solution, and for helping us iterate on this PR by testing our changes and proposing further improvements.

NlightNFotis requested a review from a team as a code owner October 21, 2024 10:09
chore: add changelog for input persistence
2a8a5cc 
Co-authored-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
NlightNFotis mentioned this pull request Oct 21, 2024
Closed
NlightNFotis changed the title Persist inputs between teh upload action and its post step. fix: persist inputs between the upload action and its post step Oct 21, 2024
jsoref approved these changes Oct 21, 2024
View reviewed changes

jsoref left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good to me. Tested it https://github.com/check-spelling-sandbox/check-spelling/actions/runs/11439466596/job/31823193138 and it worked. (The patch partially failed to apply because the source map wasn't a match to the version included in the runner cache, but that isn't a requirement -- and when I was doing my testing, I didn't include the source map file.)

Note that there are four actions that use post:. If I were maintaining this repository, I'd patch all of them otherwise, there's a risk that someone else gets tripped up that this only works in some code paths...

jsoref mentioned this pull request Oct 21, 2024
Closed
3 tasks

jsoref commented Oct 21, 2024

Copy link
Copy Markdown
Contributor

@NlightNFotis can you cherry-pick d9886b8 ? VSCode automatically wanted to remove that whitespace...

github-advanced-security AI found potential problems Oct 21, 2024
View reviewed changes
Comment thread src/analyze-action-post.ts Fixed
Comment thread src/init-action-post.ts Fixed
Comment thread src/init-action-post.ts Fixed

NlightNFotis commented Oct 21, 2024

Copy link
Copy Markdown
Member Author

@NlightNFotis can you cherry-pick d9886b8 ? VSCode automatically wanted to remove that whitespace...

Hm, I want to do that, but I get

❯ git cherry-pick d9886b85848afc58f19094c760535a6dd1a8cf77
fatal: bad object d9886b85848afc58f19094c760535a6dd1a8cf77

I think the original branch with this change has been deleted, so it cannot find the commit. I also looked into your fork and I cannot find the branch there.

Would it be possible to extract it as a patch and post it here as a comment? @jsoref

jsoref commented Oct 21, 2024

Copy link
Copy Markdown
Contributor

You can grab it from https://github.com/github/codeql-action/commit/d9886b85848afc58f19094c760535a6dd1a8cf77.patch

Remove trailing whitespace from CHANGELOG
0c7697c 
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

NlightNFotis commented Oct 21, 2024

Copy link
Copy Markdown
Member Author

@jsoref Done! Thanks again for your time and effort helping us get to a good state in our code! Your input is greatly appreciated.

henrymercer reviewed Oct 21, 2024
View reviewed changes

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I am not sure whether this is happening due to a bug or an intended omission in the mechanisms for passing inputs, but the fix LGTM.

Comment thread CHANGELOG.md Outdated
Comment thread src/actions-util.ts Outdated
Comment thread src/analyze-action.ts Outdated
Comment thread src/init-action-post.ts Outdated
Comment thread src/init-action.ts Outdated
Comment thread src/start-proxy-action-post.ts Outdated
Comment thread src/start-proxy-action.ts Outdated
Comment thread src/upload-sarif-action.ts
Comment thread src/upload-sarif-action-post.ts
review-comments: add more descriptive comments to various persist/res…
9bc4ee1 
…toreState callsites.

Co-authored-by: Henry Mercer <henrymercer@github.com>
Co-authored-by: Fotis Koutoulakis <nlightnfotis@github.com>

NlightNFotis commented Oct 21, 2024

Copy link
Copy Markdown
Member Author

@henrymercer Thank you for the review, I have integrated all of the changes in 9bc4ee1. Would you please have another look at this one?

henrymercer approved these changes Oct 21, 2024
View reviewed changes

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks @jsoref, @chrisgavin, and @NlightNFotis!

NlightNFotis merged commit b15a5b5 into main Oct 21, 2024
NlightNFotis deleted the persist-inputs branch October 21, 2024 15:30
This was referenced Oct 22, 2024
Merged
Merged
iaingalloway mentioned this pull request Apr 21, 2026
Open
haines mentioned this pull request May 10, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

henrymercer henrymercer approved these changes
+1 more reviewer
jsoref jsoref approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

upload-sarif post-action step failed: Input required and not supplied: token

5 participants

Back | FazBrowse Home | New Git URL