[BEAM-13] Add JmsIO #299
[BEAM-13] Add JmsIO #299
Conversation
|
This is a first draft for discussion and improvements. I will improve with new commits. R: @davorbonaci |
| <dependency> | ||
| <groupId>org.apache.geronimo.specs</groupId> | ||
| <artifactId>geronimo-jms_1.1_spec</artifactId> | ||
| <version>1.1.1</version> |
There was a problem hiding this comment.
Move version to the top-level?
There was a problem hiding this comment.
As it's JMS specific, I think it makes sense to keep it local to the IO. WDYT ?
|
I'll leave the rest of the review for @dhalperi. |
|
Rebased and updated to move the dependencies definition in the top level pom. About the license, the JmsIO dependencies are Apache projects (Apache ActiveMQ and Apache Geronimo), so it's already under Apache license. |
| <groupId>org.apache.activemq</groupId> | ||
| <artifactId>activemq-kahadb-store</artifactId> | ||
| <version>${activemq.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
I'm a little confused about this. I would think that these should just be dependencies of the JMS module rather than the entire project.
There was a problem hiding this comment.
I agree: it's what I proposed first. Davor said he would prefer on top level if some other parts use it.
However, as we did in Kafka, I would keep it "local" to the JMS IO module.
I can revert this.
|
[update: Per discussion this morning, JB is still working on watermarks.] |
|
Jenkins failure is not related to the PR: it failed on archetype when retrieving the maven-archetype-plugin from Central. |
| @@ -34,6 +34,7 @@ | |||
| public class JmsCheckpointMark implements UnboundedSource.CheckpointMark { | |||
|
|
|||
| private final List<Message> messages = new ArrayList<>(); | |||
| private long oldestPendingTimestamp = System.currentTimeMillis(); | |||
There was a problem hiding this comment.
- Suggest storing this as a Joda-time
Instant. - Suggest using
BoundedWindow.TIMESTAMP_MIN_VALUEas this is earlier than all timestamps. If you boot up and read old messages (beforenow), you don't want to mark the first few messages as LATE!
…estamp as watermark
…he visibility on the checkpoint mark
|
Merged! |
| <relativePath>../pom.xml</relativePath> | ||
| </parent> | ||
|
|
||
| <artifactId>jms</artifactId> |
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
JmsIO (unbounded source and sink).