Add more metadata in Jenkins webhook and resolve multiple triggers.

mohamicorp / stash-jenkins-postreceive-webhook

Webhook used to notify Jenkins when commits are made to Stash

Other

138 stars 98 forks source link

Add more metadata in Jenkins webhook and resolve multiple triggers. #71

Closed loa closed 10 years ago

loa commented 10 years ago

NOTICE: Active development, not ready for merge

Add branch and sha1 to Jenkins webhook.

loa commented 10 years ago

Discoveries:

Webhook's are sent when branches are deleted Before when neither sha1 or branch name are sent the master branch would probably be triggered. With this change Jenkins will fail trying to checkout sha1 = 0000..00 from branch that doesn't exists.

@lndbrg thought this should be implemented in shouldDeliverNotification with configuration that by default don't send webhooks on deletion of branches. In current pull-request I will only block those notifications.

loa commented 10 years ago

So I'm trying to fix the issues that we have with this plugin.

Double triggers on branches (when pull-request is open) #63, #66
Send along sha1 to be able to rebuild job in jenkins

Is there a reason for triggering webhooks on handleEvent? These changes will be triggered by onRefsChangedEvent any way so it feels redundant.

lndbrg commented 10 years ago

@Loa & @mikesir87 it's the onPullRequestRescope method in the PullRequestEventListerner that is the culprit for some of the double notifications. When you push new stuff to a branch it will already be triggered via the branch notifier. Then stash will trigger the rescope of the pull request when the PR is updated with the new commits.

We can most likely just remove the onPullRequestRescope event listener since that is already handled.

lndbrg commented 10 years ago

The documentation for that event says: "Event that is raised when the ref for the source-branch and/or the target-branch of a pull request is updated. Note that this not necessarily mean that the list of commits that the pull request covers will change."

https://developer.atlassian.com/static/javadoc/stash/2.11.0/api/reference/com/atlassian/stash/event/pull/PullRequestRescopedEvent.html

mikesir87 commented 10 years ago

Hello all! I'm sorry I've been missing for the whole discussion. Looks like it's been fun! We just had a new little one join the family a week ago, so things have been... well... busy! :)

Give me a day or two to go over the pull request and I'll let you know. First glance looks good, but haven't pulled it local to validate tests pass, run it, etc. I appreciate all of your time and effort into making this plugin better!

(on a complete side note... I'm trying to find out who all is using the plugin, as Atlassian doesn't tell me anything. If you/your company is ok, please feel free to let me know and I'll add your listing to the "Who's Using It" section of the wiki. Thanks!)

loa commented 10 years ago

Klarna is using it, you are welcome to write that.

I just pushed some more changes.

Current state:

Branch and sha1 hash added to the webhook on refchanges and trigger build button presses (fixes #64). Configuration test button is the only one triggering webhook without this metadata.
Pull-requests events are commented out since this creates redundant / ghost builds for us (open/close/edit pull-request).

This fixes the immediate issues we have. Long-term we want to look into actually testing the pull-request branch to test the product of the merge instead of the branch itself. Probably look into how github / jenkins integration works to get Jenkins more aware of the workflow in Stash more than just triggering builds for everything.

Hope to get to work more on these things the upcoming weeks!

And welcome to the little one :horse:, current chinese year :)

dhumeniuk commented 10 years ago

University of Dayton Research Institute uses it!

mikesir87 commented 10 years ago

Hello all! Sorry it's taken me a while to get to this. With the newborn and other stuff going on at my real job, this plugin unfortunately took the backburner spot. I'll take a look at it tonight and hopefully get a release going out Monday(ish) next week.

mikesir87 commented 10 years ago

Alrighty... I've looked over the code and done some testing of it locally. Everything looks good. Will package it up and make it ready for a new release. Thanks for your work in taking care of this and for your patience with me taking a while to get to it.

loa commented 10 years ago

@mikesir87 I'm sad to tell you, but I have resigned from Klarna and won't be able to push forward with this. @mindjiver will pick up the ball when he is back from vacation since Klarna still desperately needs these changes to be completed.