You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/07/20 17:01:30 UTC
[GitHub] [beam] dranderson1117 opened a new pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
dranderson1117 opened a new pull request #12320:
URL: https://github.com/apache/beam/pull/12320
Added parsedData member variable to HL7v2Message to allow non-schematized parsing customers to use the HL7v2IO connector
With adding additional nullable member variable, a Builder was also added to the class for creating new instances.
To support this new member variable, the HL7v2MessageCoder was updated to support the ParsedData class and a coder was created for ParsedData class. In addition, 2 unit tests were added to test coder functionality and validate that the new member variable does not break existing functionality.
R: @lukecwik
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #12320:
URL: https://github.com/apache/beam/pull/12320#issuecomment-666767097
@dranderson1117 - Could someone from healthcare team do a first pass review?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] dranderson1117 closed pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
Posted by GitBox <gi...@apache.org>.
dranderson1117 closed pull request #12320:
URL: https://github.com/apache/beam/pull/12320
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] stale[bot] commented on pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #12320:
URL: https://github.com/apache/beam/pull/12320#issuecomment-714216696
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] dranderson1117 commented on pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
Posted by GitBox <gi...@apache.org>.
dranderson1117 commented on pull request #12320:
URL: https://github.com/apache/beam/pull/12320#issuecomment-662470214
refactoring some of the code - will reopen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] stale[bot] commented on pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #12320:
URL: https://github.com/apache/beam/pull/12320#issuecomment-723500428
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lastomato commented on a change in pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
Posted by GitBox <gi...@apache.org>.
lastomato commented on a change in pull request #12320:
URL: https://github.com/apache/beam/pull/12320#discussion_r470226727
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HL7v2Message.java
##########
@@ -52,16 +55,19 @@ public String toString() {
* @return the hl7v2 message
*/
public static HL7v2Message fromModel(Message msg) {
- SchematizedData schematizedData = msg.getSchematizedData();
- return new HL7v2Message(
- msg.getName(),
- msg.getMessageType(),
- msg.getSendTime(),
- msg.getCreateTime(),
- msg.getData(),
- msg.getSendFacility(),
- schematizedData != null ? schematizedData.getData() : null,
- msg.getLabels());
+ HL7v2MessageBuilder mb =
+ new HL7v2MessageBuilder(
+ msg.getName(),
+ msg.getMessageType(),
+ msg.getSendTime(),
+ msg.getCreateTime(),
+ msg.getData(),
+ msg.getSendFacility());
+ mb.parsedData = msg.getParsedData();
Review comment:
nit: please use the setters
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HL7v2Message.java
##########
@@ -77,28 +83,66 @@ public Message toModel() {
out.setCreateTime(this.getCreateTime());
out.setData(this.getData());
out.setSendFacility(this.getSendFacility());
- out.setSchematizedData(new SchematizedData().setData(this.schematizedData));
- out.setLabels(this.labels);
+ if (this.getParsedData() != null) {
+ out.setParsedData(this.getParsedData());
+ }
+ if (this.getSchematizedData() != null) {
+ out.setSchematizedData(new SchematizedData().setData(this.schematizedData));
+ }
+ if (this.getLabels() != null) {
+ out.setLabels(this.labels);
Review comment:
nit: the null checks here are redundant since the setters take nullable inputs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] stale[bot] closed pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #12320:
URL: https://github.com/apache/beam/pull/12320
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on a change in pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #12320:
URL: https://github.com/apache/beam/pull/12320#discussion_r474993006
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HL7v2Message.java
##########
@@ -19,21 +19,24 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.api.services.healthcare.v1beta1.model.Message;
+import com.google.api.services.healthcare.v1beta1.model.ParsedData;
import com.google.api.services.healthcare.v1beta1.model.SchematizedData;
import java.io.IOException;
import java.util.Map;
+import java.util.Objects;
import javax.annotation.Nullable;
/** The type HL7v2 message to wrap the {@link Message} model. */
public class HL7v2Message {
Review comment:
Why not use `@AutoValue` and `@AutoValue.Builder` to generate this data object and builder for you?
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HL7v2MessageCoder.java
##########
@@ -42,17 +43,19 @@ public static HL7v2MessageCoder of(Class<HL7v2Message> clazz) {
private static final NullableCoder<String> STRING_CODER = NullableCoder.of(StringUtf8Coder.of());
private static final NullableCoder<Map<String, String>> MAP_CODER =
NullableCoder.of(MapCoder.of(STRING_CODER, STRING_CODER));
+ private static final NullableCoder<ParsedData> PARSED_DATA_CODER =
+ NullableCoder.of(ParsedDataCoder.of());
@Override
- public void encode(HL7v2Message value, OutputStream outStream)
- throws CoderException, IOException {
+ public void encode(HL7v2Message value, OutputStream outStream) throws IOException {
Review comment:
Changing the encoding breaks pipeline update on Dataflow for streaming pipelines.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org