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

Duplicate requests to sign the CLA #136

Open
wilkinsona opened this issue Jul 20, 2016 · 23 comments
Open

Duplicate requests to sign the CLA #136

wilkinsona opened this issue Jul 20, 2016 · 23 comments

Comments

@wilkinsona
Copy link
Contributor

I've only seen it once. See spring-projects/spring-boot#3963

@Shredder121
Copy link
Contributor

Coule you look up the corresponding payloads?
Maybe the label action of spring-issuemaster triggered also an update?
Although I must admit, they are a lot of time apart..

Maybe making the handling occur on one and only one thread will solve possibilities like these?

@wilkinsona
Copy link
Contributor Author

Maybe the label action of spring-issuemaster triggered also an update?

That seems unlikely. The label was applied by Spring Issuemaster before the first comment from Pivotal Issuemaster.

@Shredder121
Copy link
Contributor

At least, you would think that something must have triggered it?
Maybe the corresponding payload could be looked up (really wish that page had a filter function, or search).

But yeah, very unlikely that it was that specifically.

@rwinch
Copy link
Collaborator

rwinch commented Jul 20, 2016

@wilkinsona Thanks for the report!

I'm not sure what would have caused this at the moment. The comments are at 2:49 am CDT & 3:49 am CDT which alludes to the fact that it is not a race condition.

Looking at https://status.github.com it appears everything has been operational.

In short, I'm not sure what caused this but we will keep it open until we get it sorted out or haven't seen it in a long time.

@wilkinsona
Copy link
Contributor Author

Here's another example of a duplicate request: spring-projects/spring-framework#1120

@snicoll
Copy link

snicoll commented Jul 26, 2016

And the second message was posted a while after the PR was closed...

@mp911de
Copy link
Collaborator

mp911de commented Jul 26, 2016

Do you have access to the event notification payload? The PR was updated at 2016-07-26T12:09:00 MESZ

@Shredder121
Copy link
Contributor

@mp911de Also note, that we don't find the existing comment.
The comment could have been updated.

@mp911de
Copy link
Collaborator

mp911de commented Jul 26, 2016

What currently bugs me is that we have two guards in the code and none of them seems to work. We usually don't update closed PR's

@wilkinsona
Copy link
Contributor Author

wilkinsona commented Jul 26, 2016

Assuming I'm looking at the right place in the code, the logic is an OR so only one of the guards needs to be wrong for the comment to be made.

@mp911de
Copy link
Collaborator

mp911de commented Jul 26, 2016

The reason is that the body has in each case the body is slightly different. The first PR has a difference in the links (https://cla.pivotal.io/sign/spring vs. https://cla.pivotal.io/sign/spring?repositoryId=spring-projects/spring-boot&pullRequestId=3963). The second PR uses different user names.
That's one riddle solved. Now I just need to find out why we still put comments on closed pull requests.

@Shredder121
Copy link
Contributor

@mp911de yeah I noticed that, thanks for looking into it.

@wilkinsona the pull request state is probably either open or closed,
Could you find the corresponding webhook payload? The pull_request property should reflect what state the pull request was in.

@mp911de
Copy link
Collaborator

mp911de commented Jul 26, 2016

I'm fixing it right now and constraining comment interaction to open pull requests only.

@wilkinsona
Copy link
Contributor Author

Could you find the corresponding webhook payload?

How do I do that?

@mp911de
Copy link
Collaborator

mp911de commented Jul 26, 2016

Go to the Repository Settings -> Webhooks & services, select https://cla.pivotal.io/github/hooks/pull_request/pivotal. You'll see in the lower section of the page Recent Deliveries

@wilkinsona
Copy link
Contributor Author

I guess that's only useful for spring-projects/spring-framework#1120? I don't have access to the repository settings for Spring Framework. @snicoll might be able to help.

@mp911de
Copy link
Collaborator

mp911de commented Jul 26, 2016

I think I found also the other reason why the second comment was added. It's a combination of unknown and the manual sync. The manual sync uses the username of the user logged into Pivotal CLA. It also uses the unknown status to propagate a sync. That are the reasons why spring-projects/spring-framework#1120 got commented twice.

@Shredder121
Copy link
Contributor

It's a combination of unknown and the manual sync.

Can it be that the repository was deleted?

@mp911de
Copy link
Collaborator

mp911de commented Jul 26, 2016

ehlxr wants to merge 1 commit into spring-projects:master from unknown repository

That's another issue here. So much coincidence in one PR.

@Shredder121
Copy link
Contributor

Haha, I believe that's called "field testing".
But yeah, it's unfortunate, so many things to go wrong.

I tried to find the corresponding event in the events API, but it's not published there unfortunately.
So the information has to come from the webhook payload I suppose.

@snicoll
Copy link

snicoll commented Jul 26, 2016

I have access to recent deliveries if someone needs to look at that.

@mp911de
Copy link
Collaborator

mp911de commented Jul 26, 2016

I fixed the discovered issues with duplicate comments. Let's keep the ticket open for a few more days. We can close it at Spring One if duplicates don't pop up anymore.

@Shredder121
Copy link
Contributor

Just wondering/verifying, S1 means SpringOne right?

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

No branches or pull requests

5 participants