Also pass file_path to the handler method

[Kami]

With this change, we also pass file_path to the handler when a new line is detected.

This makes the whole thing more useful when monitoring / tailing multiple files. Previously it wasn't possible to know to which file the line refers to inside the handler function.

I also updated the handler method call to use keyword arguments instead of a dictionary. I know this is a breaking change, but it makes things more explicit and code more introspectable.

TODO:

• Update existing tests
• Add new tests

[kvdveer]

This is really a nice idea, thanks for that.

I have a bit of a nitpick about the implementation. You've changed the arguments for handler. However, as far as I can see, the actual handler hasn't changed. Instead of changing the arguments for handler, I recommend adding a file_path entry to the original dict. That's the way SyslogHandler communicates its peer address.

[Kami]

@kvdveer No problem.

I can make that change, but I really prefer to be explicit and use kwargs over a dict "with an arbitrary structure". With a dict approach, it's hard / impossible to do the introspection, and it's hard for user to know what to expect without checking the docs and / or code.

Edit: I see the syslog issue - if I understand you correctly, syslog adds additional entries to the dict which contains the message?

[Kami]

Updated the code to use a dict, added tests cases (1bd1a63). Also cherry-picked fixes for test failures which are unrelated to my changes from my other pull request.

[kvdveer]

The idea behind the dict is that is actually is a bunch of arbitrary
data fields, which gets processed by the DSL which is the YAML pipeline.
Syslog indeed provides the fields hostname, message, and timestamp.

I see your concern with unstructured data, but changing that would be
far more involved than just changing a call in one data source. I'll
ponder it a bit in the near future.

On 03/23/2015 10:48 PM, Tomaz Muraus wrote:

Updated the code to use a dict, added tests cases (1bd1a63
1bd1a63).
Also cherry-picked fixes for test failures which are unrelated to my
changes from my other pull request.

—
Reply to this email directly or view it on GitHub
#11 (comment).

[coveralls]

Coverage decreased (-0.36%) to 86.83% when pulling adc2b9b on Kami:pass_file_path_to_the_handler into 1ba5a6c on ondergetekende:master.

[Kami]

@kvdveer Would it be possible to have a look at this again and merge it?

Right now we are still using our fork of the library and would be great if we can get rid of that and just use the official version.