-
Notifications
You must be signed in to change notification settings - Fork 20
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
Broadcaster 448 #22
Broadcaster 448 #22
Conversation
…o broadcaster-448
Hrm... Travis is getting blocked from Maven central. I'll have to investigate. |
@dannylamb I've seen that happen from time to time. I usually see it as an opportunity to get a cup of tea or coffee. When I return, it's usually better. |
@acoburn 👍 You're right. |
input.stream=broker:queue:islandora-connector-broadcast | ||
|
||
# Header that contains the recipient list | ||
recipients.header=ca.islandora.alpaca.connector.broadcast.recipients |
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.
This is just my own personal preference, but I like to use a convention more like this for naming headers: CamelClawBroadcasterRecipients
. This makes it a little easier to refer to the header in simple
expressions, since you can refer to it like this: simple("${header.CamelClawBroadcasterRecipients}")
. There certainly are camel components that use the ca.islandora.alpaca....
convention, and Fedora's JMS module uses org.fcrepo.jms.___
, but for most things that run entirely inside Camel, I've seen the pattern I recommended above.
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.
I like that. It's certainly shorter and easier on the eyes. I was checking out Fedora's messages and rolled with it when looking for a convention, since I figured the X-Islandora-BlahBlah style was more for HTTP.
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.
And you may decide on something like CamelAlpacaBroadcasterRecipients
or the like -- whatever you think best fits.
dependencies { | ||
compile group: 'org.apache.camel', name: 'camel-core', version: camelVersion | ||
compile group: 'org.apache.camel', name: 'camel-blueprint', version: camelVersion | ||
compile group: 'org.slf4j', name: 'slf4j-api', version: slf4jVersion |
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.
It's fine to leave this here, but I'd note that you're not actually using SLF4J in the code. (You may want to at some future point, but I'd probably remove this until that time comes).
* | ||
* @author Daniel Lamb | ||
*/ | ||
public class EventRouter extends RouteBuilder { |
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.
One thing that I've noticed when I generated the Javadocs for our Camel code is that I've got about a dozen EventRouter
classes. You may want to think about naming this something like BroadcastEventRouter
or similar.
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.
Good point.
from("{{input.stream}}") | ||
.routeId("MessageBroadcaster") | ||
.description("Broadcast messages from one queue/topic to other specified queues/topics.") | ||
.log("Distributing message: ${headers[JMSMessageID]} with timestamp ${headers[JMSTimestamp]}") |
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.
Here's where SLF4J could really be used. By using the .log(level, logger, message)
interface instead of the simple one here, your users will be able to have more control over the logging of these messages. Here's an example: https://github.com/fcrepo4-exts/fcrepo-camel-toolbox/blob/master/fcrepo-serialization/src/main/java/org/fcrepo/camel/serialization/SerializationRouter.java#L116
With this, you can set the logging level in Karaf (for example) to include or exclude the messages from this particular route; otherwise, you don't have quite such fine-grained control.
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.
I'll definitely update to use that instead. I certainly want users to control the logging level through Karaf if possible.
Resolves Islandora/documentation#448
See testing instructions at Islandora/documentation#457, since this requires updates to the install process.