[BEAM-716] Use AutoValue in JmsIO #1577
Conversation
|
R: @lukecwik |
|
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1--none-- |
| @AutoValue | ||
| public abstract static class Read extends PTransform<PBegin, PCollection<JmsRecord>> { | ||
|
|
||
| /** |
There was a problem hiding this comment.
Makes more sense to put javadoc on the publicly visible members - in this case, withBlah() methods
| } | ||
|
|
||
| return input.getPipeline().apply(transform); | ||
| } | ||
|
|
||
| @Override | ||
| public void validate(PBegin input) { | ||
| checkNotNull(connectionFactory, "ConnectionFactory not specified"); | ||
| checkArgument((queue != null || topic != null), "Either queue or topic not specified"); | ||
| checkNotNull(getConnectionFactory(), "ConnectionFactory not specified"); |
There was a problem hiding this comment.
Might as well to fix these error messages to be in line with those you have in MqttIO etc?
There was a problem hiding this comment.
Yup, I planed to do this in a separate PR but it makes sense to do it in this one.
There was a problem hiding this comment.
FYI, I planned to do this as part of BEAM-959 (PR related to this Jira).
|
Refer to this link for build results (access rights to CI server needed): |
[BEAM-959] Improve check preconditions in JmsIO
|
Rebased |
|
Refer to this link for build results (access rights to CI server needed): |
|
@davorbonaci @jkff do you mind to take a last review and LGTM if good ? I will merge as soon as you give my the green light. |
|
Huge LGTM from me. I think the title of the PR is selling it short. There's plenty of other improvements here, such as Javadoc documentation, other cleanup, etc. Thanks @jbonofre! |
|
Thanks ! Merging. |
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.