Automatically create backport PR if oauth token is set #175
Conversation
- Provide instructions on how to generate the personal access token, and how to set it as an environment variable. - Add gidgethub and requests as dependencies.
|
python/cpython#3224 was created via this method, using my own personal access token. |
|
I have read the readme, but it is not clear to me what 'create automatically' means -- what will happen, when and where -- and what a token on my machine has to do with it. |
|
Thanks @terryjreedy. The idea is instead of opening a web browser where we have to click the 'Create pull request button', cherry picker will create the PR. So no web browser will be opened at all. The token is needed by GitHub API as a way to authenticate your GitHub account. |
|
Thanks for explaining. Since I like to have the PR open in my browser, and clicking the button is an easy way to get that, I will probably pass. |
|
No prob :) It's backward compatible, so the previous way still works as is. Potentially we can use this for the cherry-picking bot, where the backport will happen automatically after we merge the PR on |
cherry_picker/readme.rst
Outdated
| Personal Access Token (optional) | ||
| -------------------------------- | ||
|
|
||
| ``cherry_picker`` can automatically create the PR, if the ``GH_AUTH`` token is set |
There was a problem hiding this comment.
Is this considered safe and best practice for storing an auth token like this? I honestly don't know, which is why I'm asking. 😄
There was a problem hiding this comment.
I don't really know 😅
This is really so I can use it for the cherry picker bot later.
Maybe I should keep this as hidden and secret functionality, and not mention it at all in the readme?
There was a problem hiding this comment.
Yeah, either don't document it or be clear that one should be careful about how that environment variable is stored.
There was a problem hiding this comment.
Ok I'm going with "not documenting it" since it's easier to do than coming up with warning messages 😅
|
I didn't expect the Spanish Inquisition! |
| @@ -6,9 +6,13 @@ | |||
| import subprocess | |||
| import webbrowser | |||
| import sys | |||
| import requests | |||
|
|
|||
| from gidgethub.sansio import create_headers | |||
There was a problem hiding this comment.
Not important, but you shifted from importing modules to importing objects with this import line.
| else: | ||
| self.open_pr(self.get_pr_url(https://github.com/base_branch,%20head_branch)) | ||
|
|
||
| def create_gh_pr(self, base_branch, head_branch, commit_message): |
There was a problem hiding this comment.
If you're already checking for the envvar above, would it make sense to make it a required argument and just pass it in explicitly?
There was a problem hiding this comment.
Ok, I made the token a required argument.
| @@ -8,7 +8,7 @@ | |||
| import sys | |||
| import requests | |||
|
|
|||
| from gidgethub.sansio import create_headers | |||
| import gidgethub | |||
There was a problem hiding this comment.
This should either be import gidgethub.sansio or from gidgethub import sansio; the fact that the package has an import is undocumented.
There was a problem hiding this comment.
Changed tofrom gidgethub import sansio
| else: | ||
| self.open_pr(self.get_pr_url(https://github.com/base_branch,%20head_branch)) | ||
|
|
||
| def create_gh_pr(self, base_branch, head_branch, commit_message): | ||
| request_headers = create_headers(self.username, oauth_token=os.getenv("GH_AUTH")) | ||
| def create_gh_pr(self, base_branch, head_branch, commit_message, gh_auth): |
There was a problem hiding this comment.
Is there any worry the order of these arguments will be non-obvious? I.e. should any of them be keyword-only arguments?
There was a problem hiding this comment.
Hmm, ok I made commit_message and gh_auth as keyword only arguments.
made commit_message and gh_auth keyword only args in create_gh_pr
gidgethubandrequestsas dependencies.Closes #100