Get rack/parser working #5
Conversation
dhollinger
requested review from
alexjfisher,
bastelfreak,
binford2k and
rnelson0
lib/parsers/webhook_json_parser.rb
Outdated
| ).sub('refs/heads/', '') rescue nil | ||
| end | ||
|
|
||
| def parse_deleted?(data) |
There was a problem hiding this comment.
Ideas for how to better handle this would be awesome as this is really ugly.
lib/parsers/webhook_json_parser.rb
Outdated
| data['refChanges'][0]['type'] rescue nil || # Stash/Bitbucket Server | ||
| data['after'] rescue nil # Gitlab | ||
| ) | ||
| return false unless [ |
There was a problem hiding this comment.
Shouldn't this just be ![...].include?(branch_deleted) so you always return a boolean?
There was a problem hiding this comment.
@ekohl Without the leading ! ?
There was a problem hiding this comment.
Oh yes, the false unless was too much for me in the early morning :)
lib/parsers/webhook_json_parser.rb
Outdated
|
|
||
| def parse_deleted?(data) | ||
| branch_deleted = ( | ||
| data['deleted'] rescue nil || # Github |
lib/parsers/webhook_json_parser.rb
Outdated
|
|
||
| def parse_branch(data) | ||
| ( | ||
| data['ref'] rescue nil || # github & gitlab |
| @@ -87,13 +87,7 @@ class PuppetWebhook < Sinatra::Base | |||
| data = JSON.parse(decoded, quirks_mode: true) | |||
There was a problem hiding this comment.
This might be for a future improvement but I'd expect that if there's a WebhookJsonParser that it'd do all of it.
There was a problem hiding this comment.
I do believe that Rack::Parser is already doing the work. We just need to update our code to add what we need to the params Rack::Parser builds
Got the Rack::Parser code and custom parser working. Moved the branch name parsing into its own method that is call from the call method. Added a parse_deleted? method as well. The Sinatra params will now contain params called branch(String) and deleted(Boolean). Code in the payload POST route has been updated to use these params instead of running the parsing logic itself. I also created a new directory call "parsers" under "lib" and moved webhook_json_parser.rb there. The class's namespace has been updated to Sinatra::Parsers::WebhookJsonParser. All calls into that file/class have been updated as well. All "puts" in the code have been removed as they were there for Dev troubleshooting purposes. Updated .gitignore to ignore the .log files in the logs directory, but also added a .keep file inside the logs directory so users/devs don't have to create it themselves. Gemfile.lock was updated after a bundle install and bin/puppet_webhook had its permissions changed to 755. All other work is related to the JSON parsing with Rack::Parser
alexjfisher
force-pushed
the
json_parsing
branch
from
bd2ab88 to
63a06ed
Compare
|
I fixed up the indentation and the parse_deleted? return before merging. |
| @@ -1,6 +1,5 @@ | |||
| *.gem | |||
| webhook.lock | |||
| .ruby_version | |||
| *.log | |||
| logs/ | |||
| logs/*.log | |||
There was a problem hiding this comment.
This should log to /var/log/puppet-webhook or the like, rather than in the gem directory.
There was a problem hiding this comment.
Agreed, I just haven't gotten around to it yet.
Got the Rack::Parser code and custom parser working. Moved the branch
name parsing into its own method that is call from the call method.
Added a parse_deleted? method as well. The Sinatra params will now
contain params called branch(String) and deleted(Boolean). Code in the
payload POST route has been updated to use these params instead of
running the parsing logic itself.
I also created a new directory call "parsers" under "lib" and moved
webhook_json_parser.rb there. The class's namespace has been updated to
Sinatra::Parsers::WebhookJsonParser. All calls into that file/class have
been updated as well.
All "puts" in the code have been removed as they were there for Dev
troubleshooting purposes.
Updated .gitignore to ignore the .log files in the logs directory,
but also added a .keep file inside the logs directory so users/devs
don't have to create it themselves.
Gemfile.lock was updated after a bundle install and bin/puppet_webhook
had its permissions changed to 755.
All other work is related to the JSON parsing with Rack::Parser