[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Brett C. added the comment: Can this be closed? ___ PSF Meta Tracker ___

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Ezio Melotti added the comment: This happened again in http://bugs.python.org/issue29554 -- not sure if someone was playing with the payloads or if something else triggered the duplication. ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Ezio Melotti added the comment: I noticed that it's possible to add the same PR multiple times to the same issue, see e.g. http://bugs.python.org/issue29546 This probably happened because the payload was re-sent manually, and shouldn't otherwise be an issue, so we might not have to fix it (but

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Brett C. added the comment: I've tested this and it seems to be working. ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Brett C. added the comment: LGTM ___ PSF Meta Tracker ___

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: I tested the latest patch on the my repository and it is working properly. :) ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Maciej Szulik added the comment: I'm uploading the current version of the patch, it's still not fully reviewed, I've left a few TODO's here and there for what should be double checked. ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Maciej Szulik added the comment: LGTM ___ PSF Meta Tracker ___

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: I have updated the patch with all three changes ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: Oops. Got it now. I will include the changes. ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: new tests: - secret key missing - Github event missing in header - non json body - multiple references in PR body - CREATE_ISSUE is not set and no issue is reference in PR - if github field of user is set, then PR/issue is assigned to that user or else anonymous

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Maciej Szulik added the comment: > Sorry. I didn't get you. You mean, same tests for PR body, right? Yeah, exactly that. I'd like to see both body and title tests. Sorry I was initially noting down my thoughts and apparently forgot to "normalize" that part of the sentence ;) > I was

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: > * multiple fixes both in title like you have in pullrequestevent3.txt but also in title Sorry. I didn't get you. You mean, same tests for PR body, right? > Is vulnerable to other exceptions, I'm a lazy person and I test your code with following > command: >

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: Should I throw Reject exception if 'SECRET_KEY' is not set as an environment variable too? ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Maciej Szulik added the comment: roundup/pull_request.py: > key = os.environ['SECRET_KEY'] If this env is missing the code throws KeyError, which is not nice. It would be better to check if it exists and fail nicely, informing about the problem. > def testPullRequestEventForTitle(self): >

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: I have added few things to this updated patch: - load JSON while extracting - user who creates GitHub PR will be assigned as PR author on bpo if we find github name on bpo or else "anonymous" - if issue is not referenced in PR title or body and an environment

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: > Maybe put the code in with a switch so we can turn it on and off? I'm pretty sure we're going to want it, but not positive. Sorry. I didn't get you. What do you want to turn on and off? ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

R David Murray added the comment: Maybe put the code in with a switch so we can turn it on and off? I'm pretty sure we're going to want it, but not positive. ___ PSF Meta Tracker

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Anish Shah added the comment: Thank you for the review. I will update the patch. @David I can create a new issue if there's no issue number mentioned. But as Brett said that someone might forget to add issue number. I'm currently checking for issue id in PR title, body and issue comments. So,

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

R David Murray added the comment: Yes, I was assuming we'd post to the right account when we can determine it. It was only github accounts with no linkage where we'd need a 'generic' user for posting. And we *will* get PRs from people who won't open bugs.python.org accounts, even if asked

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Maciej Szulik added the comment: @anish.shah I still don't see my comments from previous review addressed. Additionally, since issue 586 has changed the schema you should update it here as well. @David: > Hmm. Good question. We could use a specific user to create such issues. The other

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

Brett C. added the comment: Another issue is that what happens if there is an issue, but the person simply forgot to include it? Now we have an extra issue that will need to have its superceder set and the issue closed. Eventually we could have a bot that automatically added a comment that

[Tracker-discuss] [issue589] Add GitHub PR to b.p.o issues using GitHub webhooks (depends on issue586)

R David Murray added the comment: Hmm. Good question. We could use a specific user to create such issues. As for noise...we do typo issues now, so if we have an increased volume of them I think that just means we're fixing more typos. I don't really see the problem with that, since