-
Notifications
You must be signed in to change notification settings - Fork 272
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
Add GitHub triage helper app #2505
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this PR!
## Next Steps | ||
|
||
The current implementation is subject to hourly rate limits, which means it error out if too many users attempt to run it within a given hour window. | ||
To make the implementation more reliable, we should register the app with a GitHub API token so that our hourly rate limit can be increased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we register the app with a GitHub API token before merging this PR? Is it a quick update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think it will take some time as I haven't done it before :). It's also a little tricky due to the following reasons:
-
I'm not certain that the GitHub token should live under my GitHub account, so I'll need to find a more general user group and register under that. It's not immediately obvious to me what that more general user group will be.
-
Currently the azure app is under my account, I'll need to find a more general user group there too and deploy it there, so we all have management access to it.
I think that setting up these things will have me moving the API token around until we find the final home for this app.
I also presume that the GitHub token stuff would not live in this repo (because that's a secret) so it would have to be published as an app setting on the corresponding azure app. I assume that the only change needed to this code is to conditionally read the token from the app settings (if it exists) and append it to the URL in the right place.
For now, given that the app may be executed locally and still provide some automation + work around the rate limit that way (if we hit the limit on azure, we can just run it locally), I think it would be worth merging this in at the very least as a local tool.
But no strong feelings on this. If you would strongly prefer that we wait until the app is fully registered on Azure, then we can hold off on this. It will just probably take me some time, maybe a few weeks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, that makes sense! Thanks for the explanation! I'm fine with merging this now so I'll approve the PR, but can you create a GitHub issue with the information in your comment so we can track it somewhere? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Created that issue here: #2533
Co-authored-by: Varshitha Bachu <vabachu@microsoft.com>
Our GitHub issue triage process spans several repos, meaning that normally we need to do the manual work of collect all open issues with the "needs: triage" label across all DF repositories.
This PR implements a Functions app that does this work for us. It utilizes the GitHub API to automatically query all "needs: triage" labels across all our repos. It can be executed both locally and on Azure.