Skip to content
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

Merged
merged 8 commits into from
Dec 20, 2016
Merged

Broadcaster 448 #22

merged 8 commits into from
Dec 20, 2016

Conversation

dannylamb
Copy link
Contributor

@dannylamb dannylamb commented Dec 13, 2016

Resolves Islandora/documentation#448

See testing instructions at Islandora/documentation#457, since this requires updates to the install process.

@dannylamb
Copy link
Contributor Author

Hrm... Travis is getting blocked from Maven central. I'll have to investigate.

@acoburn
Copy link
Contributor

acoburn commented Dec 13, 2016

@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.

@dannylamb
Copy link
Contributor Author

@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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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
Copy link
Contributor

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 {
Copy link
Contributor

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.

Copy link
Contributor Author

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]}")
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@whikloj whikloj merged commit 2c578ca into Islandora:master Dec 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants