Skip to content

A few fixes for the auto-cherry-pick script #19307

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

Merged
merged 2 commits into from
Jun 14, 2023
Merged

Conversation

thejcannon
Copy link
Member

@thejcannon thejcannon commented Jun 14, 2023

A few fixes:

  • Stop using if: always(). As I'm understanding it, this will force the job to run, even if prerequisites didn't run (I'm thinking this is for, like critical teardown jobs/steps). Instead use if: success() || failure(), which is suggested in the docs. As I understand there are (at least?) 4 states: success, failure, skipped, and cancelled.
  • Used upstream github-script since we no longer talk to the web
  • Pass a dummy PAT
  • Add a new test for PR event
  • Add a new xfail test that should be testing the bugfix, but unfortunately act has a bug

So, can't test that this fixes it, but the suggestion comes from GitHub directly: https://docs.github.com/en/actions/learn-github-actions/expressions#always

@thejcannon thejcannon requested a review from huonw June 14, 2023 02:39
@thejcannon thejcannon added the category:internal CI, fixes for not-yet-released features, etc. label Jun 14, 2023
steps:
- name: Check out code
uses: actions/checkout@v3
- run: npm install ./build-support/cherry_pick
- name: Run Script
# NB: See https://github.com/actions/github-script/pull/397
uses: thejcannon/github-script@7ed0cc648959192a3868f0de9862110d7922b7cc
uses: actions/github-script@v6
with:
github-token: ${{ secrets.WORKER_PANTS_CHERRY_PICK_PAT }}
allow-empty-token: true
Copy link
Contributor

@huonw huonw Jun 14, 2023

Choose a reason for hiding this comment

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

Is this allow-empty-token now unneeded (here and above)?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Approved conditional on satisfying yourself that the root cause of the token-related error is fixed here, and we're not just papering over it in the case of a PR without cherry-picks. (I think the change to avoid always makes sense though, even if there's additional changes required.)

@thejcannon
Copy link
Member Author

Yeah sorry, I shouldn't make PRs before bed 😂

I'll switch the PR description to be accurate. The changing to upstream github-script is the actual bugfix. The always() also has to be in this PR (or an earlier one) so we don't start commenting on every PR.

@thejcannon thejcannon changed the title Don't use if: awlays() in GHA A few fixes for the auto-cherry-pick script Jun 14, 2023
@thejcannon thejcannon enabled auto-merge (squash) June 14, 2023 14:22
@thejcannon thejcannon merged commit d56a6d9 into main Jun 14, 2023
@thejcannon thejcannon deleted the jcannon/fixcherries branch June 14, 2023 15:51
@thejcannon
Copy link
Member Author

Seems to have worked 😄 https://github.com/pantsbuild/pants/actions/runs/5269341468

@thejcannon
Copy link
Member Author

OK poking around the GH API, there seem to be at least the four states mentioned above. 👍

@huonw
Copy link
Contributor

huonw commented Jun 15, 2023

The changing to upstream github-script is the actual bugfix

👍

The always() also has to be in this PR (or an earlier one) so we don't start commenting on every PR

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto cherry picker fails with "Token passed to createTokenAuth is not a string"
2 participants