-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds an EventBuilder to dissociate the DSL/builder from the client #84
Adds an EventBuilder to dissociate the DSL/builder from the client #84
Conversation
+1 for this PR, i also want to be able to construct events without relying on a RiemannClient. |
@aphyr I am broadly +1 with test coverage but am not a regular user of the client. Do you have any thoughts? |
I've added more tests and fixed a bug. The |
// health -> good | ||
assertEquals("health", eventAttributes.get(2).getKey()); | ||
assertEquals("good", eventAttributes.get(2).getValue()); | ||
// health -> good |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
network_zone -> dmz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
Actually I don't think these comments are really useful, I will remove them.
LGTM |
@jamtur01 Ok to merge this PR ? |
Thanks! |
The
EventDSL
is useful but at the same time the class is tightly coupled with the RiemannClient and the constructor is usingjava.net.InetAddress.getLocalHost()
(which can make call to the DNS resolver... see #44).Also in our context, we have a dedicated host that send events to Riemann (so the call to
java.net.InetAddress.getLocalHost()
is useless).Using the low-level builder
Proto.Event.newBuilder()
as an alternative is a bit rough (at least for a newcomer).As a result I've decided to create a new class named
EventBuilder
who's only job is to ease the creation of anEvent
. To avoid code duplication,EventDSL
is usingEventBuilder
.This change does not introduce a breaking change since the API of the
EventDSL
remains the same.I've started to write one unit test (and I planned to write more and also add documentation) but before spending more time on it, I would like to hear from you. Are you OK with this change ?