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

use refactored find a pat script to pass token to slash command dispatch action via env #16321

Merged

Conversation

zzztimbo
Copy link
Contributor

@zzztimbo zzztimbo commented Sep 4, 2022

Summary

This reinstates the work of @tuliren's PR by using a refactored find_non_rate_limited_PAT that writes the token to env (not base64 encoded).

Tests

@zzztimbo zzztimbo changed the title use refactored find a pat script to pass token to slash command dispatch action use refactored find a pat script to pass token to slash command dispatch action via env Sep 4, 2022
Copy link
Contributor

@supertopher supertopher left a comment

Choose a reason for hiding this comment

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

This is a great fix Tim. Appreciate the test runs in the PR as well

ship it!

- name: Slash Command Dispatch
id: scd
uses: peter-evans/slash-command-dispatch@v2
with:
token: ${{ secrets.SUPERTOPHER_PAT }}
token: ${{ env.PAT }}
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a real solid move @zzztimbo

are all env vars hidden by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Github recognizes the PAT as a secret, but probably not other strings?
image

Copy link
Contributor

@c-p-b c-p-b left a comment

Choose a reason for hiding this comment

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

LGTM, but with the concern raised of token reusage tracked in https://github.com/airbytehq/airbyte-cloud/issues/2641

Copy link
Contributor

@git-phu git-phu left a comment

Choose a reason for hiding this comment

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

nice!

also appreciate the links to the test runs!

@zzztimbo zzztimbo merged commit a4621f8 into master Sep 6, 2022
@zzztimbo zzztimbo deleted the tim/use-env-to-pass-pat-to-slash-command-dispatcher branch September 6, 2022 16:30
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
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.

4 participants