Add Event method_missing to give better error messages for #[] and #[]= #6045
Conversation
|
I like this idea. Since it's a debatable thing, I vote for getting a second or third LGTM. That said, LGTM |
| end | ||
| if /^\[\]=$/.match(method_name.to_s) | ||
| raise NoMethodError.new("Direct event field references (i.e. event['field'] = 'value') have been disabled in favor of using event get and set methods (e.g. event.set('field', 'value')). Please consult the Logstash 5.0 breaking changes documentation for more details.") |
There was a problem hiding this comment.
we are using an almost identical duplicate string, maybe create a string const?
There was a problem hiding this comment.
OK - a frozen set, perhaps.
|
I am good with this approach, solves the problem and will not interfere in normal operation. |
|
@colinsurprenant |
|
Is is actually not important to have the jar loaded or not ... this is in the Ruby Event ... this is the purpose of |
| @@ -31,7 +31,24 @@ def shutdown?; false; end; | |||
| SHUTDOWN = ShutdownEvent.new | |||
| end | |||
|
|
|||
| module LogStash class Event | |||
There was a problem hiding this comment.
this is nitpicking but why not define the class Event in the previous module LogStash definition ?
|
Great! :D if all tests are passing I'm LGTM |
|
got 2 LGTM - squashing |
Change strings and regexs to CONSTANTS. move method missing code to event.rb doh - put Event class in already defined Logstash module
guyboertje
force-pushed
the
fix/logstash-filter-ruby/29
branch
from
9a736b5
to
38517d7
Compare
Change strings and regexs to CONSTANTS. move method missing code to event.rb doh - put Event class in already defined Logstash module Fixes #6045
Change strings and regexs to CONSTANTS. move method missing code to event.rb doh - put Event class in already defined Logstash module Fixes #6045
Reviewer:
I elected to add a method_missing handler for the Event class.
I chose not to do this in the JRuby Extension because IMO it will be easier to remove it later.
If it seems like we need to keep it for all time, we can move it to the JRuby Extension.