Added support for BitBucket merge hooks #193
Conversation
| @@ -108,7 +114,13 @@ class Server < Sinatra::Base | |||
| data = JSON.parse(decoded, :quirks_mode => true) | |||
|
|
|||
| # github sends a 'ref', stash sends an array in 'refChanges' | |||
| branch = ( data['ref'] || data['refChanges'][0]['refId'] ).sub('refs/heads/', '') | |||
| if $config['git_provider'] == 'github' | |||
| branch = ( data['ref'] || data['refChanges'][0]['refId'] ).sub('refs/heads/', '') | |||
There was a problem hiding this comment.
This code actually is not just github, its github, gitlab and stash
There was a problem hiding this comment.
Yeah I gathered that when I looked at my Gitlab environment, I needed a variable name to standardize on, so I chose GitHub as in "GitHub compliant," figured githublabstash would be too messy :) I am certainly open to suggestions of change.
There was a problem hiding this comment.
So Basically I would want to make this a required option for all services or fully automatic like it is now, My assumtion is you could just move your code data['pullrequest_merged']['destination']['branch']['name'] to the end of the || conditionals and this would work out of the box without the need for the param to exist.
There was a problem hiding this comment.
That would work. I was concerned with the logic clause becoming unmanageable over time. Either way actually works for me. I can resubmit.
After discussion with Zack, I moved the JSON data changes for pulling branch name and repository name back into the logic evaluation inline. I removed the references to the git_provider configuration setting and pulled it from params and the webhook config.
|
I updated the branch to merge in with just the inline evaluation for the JSON. |
|
Don't merge this yet, it doesn't work in practical testing. So it seems like the following is occurring; when the first attempt to access the JSON parsed output in the data variable and it's not found (because it's Bitbucket and doesn't have the GitHub format) ruby is throwing an exception and it's terminating the code at that point. I think we're going to have to add exception handling and rescue it out to the next parsing attempt for each body format. |
|
Confirmed, this is part of an issue with hash and nil items. Because data['repository'] returns nil, then data['repository']['name'] returns a NoMethodError and exits the code; the reason it worked for github vs. stash in the payload portion is that data['ref'] returns nil so it just goes to the next OR clause, in the case of BitBucket, the stash data access throws the NoMethodError and exits the code. I am postulating the best way to handle this. |
In the OR logic clause for parsing the JSON hash, the code was throwing NoMethodError for accessing hash levels deeper than the first that did not exist. This was causing the code to prematurely exit. I added an inline rescue to rewrite it as nil to progress to the next step in the OR logic clause wherever this could happen.
|
Turns out I was able to just add an inline rescue that set nil in the event that it throws the error. This should work. |
|
It appears this update will only handle bitbucket pull request merge events. I updated our local webhook code to handle bitbucket push events by referring to this key for the branch/environment name: |
|
@dharmabruce can you submit a tested PR with your changes and I will get it merged after @ayohrling can also confirm it working - Thanks |
…et_pullmerge Added support for BitBucket merge hooks
|
Tested PR submitted: https://github.com/acidprime/r10k/pull/227 |
Added support for git_provider configuration setting for the webhook to allow additional payload formats for the module and payload webhook endpoints. Defaulted to the current GitHub standard.