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 2022/06/03 16:46:16 UTC

[GitHub] [beam] fbeevikm opened a new pull request, #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

fbeevikm opened a new pull request, #17741:
URL: https://github.com/apache/beam/pull/17741

   **Please** add a meaningful description for your change here
   
   Add FhirBundleWithMetadata in executebundles method so that we can pass additional information like message id.
   FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. Hl7 messageId) to be executed on the intermediate FHIR store.
   
   Make FhirResourcePagesIterator constructor public. Whistle plugins calls this constructor.
   ------------------------
   
   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).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem closed pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem closed pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.
URL: https://github.com/apache/beam/pull/17741


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883954883


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1492,20 +1513,23 @@ private void parseResponse(ProcessContext context, String inputBody, HttpBody re
             // 20X's are successes, otherwise failure.
             if (statusCode / 100 == 2) {
               success++;
-              context.output(Write.SUCCESSFUL_BODY, entry.toString());
+              context.output(
+                  SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), entry.toString()));
             } else {
               fail++;
               context.output(
-                  Write.FAILED_BODY,
+                  FAILED_BUNDLES,
                   HealthcareIOError.of(
-                      inputBody, HealthcareHttpException.of(statusCode, entry.toString())));
+                      FhirBundleResponse.of(context.element()),
+                      HealthcareHttpException.of(statusCode, entry.toString())));
             }
           }
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(success);
           EXECUTE_BUNDLE_RESOURCE_ERRORS.inc(fail);
         } else if (bundleType.equals(BUNDLE_RESPONSE_TYPE_TRANSACTION)) {
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(entries.size());
-          context.output(Write.SUCCESSFUL_BODY, bundle.toString());
+          context.output(
+              SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), bundle.toString()));

Review Comment:
   done



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] msbukal commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
msbukal commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r881954049


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7

Review Comment:
   I think we should change the name from "FhirBundleWithMetadata" to something more generic, since now ExecuteBundle MUST accepts this as input. How about "FhirBundleParameter", similar to the input to search (FhirSearchParameter).
   
   



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7
+ * message path) in JSON format to be executed on any FHIR store. *
+ */
+@DefaultCoder(FhirBundleWithMetadataCoder.class)
+@AutoValue
+public abstract class FhirBundleWithMetadata {
+
+  static Builder builder() {
+    return new AutoValue_FhirBundleWithMetadata.Builder();
+  }
+
+  /**
+   * String representing the source of the Bundle to be written. Used to pass source data through
+   * the ExecuteBundles PTransform.
+   */
+  public abstract String getMetadata();

Review Comment:
   Remove occurrences of "source" -> "metadata", since it doesn't have to be the source.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7
+ * message path) in JSON format to be executed on any FHIR store. *
+ */
+@DefaultCoder(FhirBundleWithMetadataCoder.class)
+@AutoValue
+public abstract class FhirBundleWithMetadata {
+
+  static Builder builder() {
+    return new AutoValue_FhirBundleWithMetadata.Builder();
+  }
+
+  /**
+   * String representing the source of the Bundle to be written. Used to pass source data through
+   * the ExecuteBundles PTransform.
+   */
+  public abstract String getMetadata();
+
+  /** FHIR R4 bundle resource object as a string. */
+  public abstract String getBundle();
+
+  /** HTTP response from the FHIR store after attempting to write the Bundle method. */
+  public abstract String getResponse();

Review Comment:
   It seems confusing to include the "response" field in the input to an ExecuteBundle request. Especially since the only 2 constructors are "bundle only" or "bundle, metadata, and response". How would you create a bundle with metadata as input?
   
   I think the solution here should be to make the response of the ExecuteBundle DoFn it's own "FhirBundleResponse" class, which can contain a FhirBundleWithMetadata/FhirBundleParameter inside it (or just the two fields, this might be easier regarding serialization, either is fine) as the "source".



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7
+ * message path) in JSON format to be executed on any FHIR store. *

Review Comment:
   Comment nits:
   * it's -> its
   * any FHIR store -> a FHIR store (specifying any can be confusing, it's the specific FHIR store that a user passes in)
   * change the example from "HL7v2 message path" to something more generic, eg. the Pubsub Notification ID (since most executes come from reads), don't need to bring up another modality.
   * mention the metadata seperately: emphasize the main purpose first:
   `FhirBundleWithMetadata represents a FHIR bundle in JSON format to be executed on a FHIR store`
   then mention that the passed in metadata will be returned in the response, and can be used for tracking metadata such as, for example, the source.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1496,16 +1517,22 @@ private void parseResponse(ProcessContext context, String inputBody, HttpBody re
             } else {
               fail++;
               context.output(
-                  Write.FAILED_BODY,
+                  FAILED_BUNDLES,
                   HealthcareIOError.of(
-                      inputBody, HealthcareHttpException.of(statusCode, entry.toString())));
+                      context.element(), HealthcareHttpException.of(statusCode, entry.toString())));
             }
           }
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(success);
           EXECUTE_BUNDLE_RESOURCE_ERRORS.inc(fail);
         } else if (bundleType.equals(BUNDLE_RESPONSE_TYPE_TRANSACTION)) {
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(entries.size());
-          context.output(Write.SUCCESSFUL_BODY, bundle.toString());
+          context.output(

Review Comment:
   In the BATCH case above (line 1516) we output to Write.SUCCESSFUL_BODY, which the PTransform no longer returns, since the output tags are only:
   
   `.withOutputTags(SUCCESSFUL_BUNDLES, TupleTagList.of(FAILED_BUNDLES)));`
   
   This means that nothing will be returned in the batch case. If you run the integration tests (can comment `Run Java PostCommit`) it'll fail. 
   
   Please fix.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/GetStringHealthcareIOErrorFn.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import org.apache.beam.sdk.transforms.DoFn;
+
+/**
+ * ParDo function that transforms a HealthcareIOError for a FhirBundleWithMetadata to an error with
+ * the body (string) as the data resource, for backwards compatibility.
+ */
+public class GetStringHealthcareIOErrorFn

Review Comment:
   I agree with Mackenzie's comment: This shouldn't be it's own class/DoFN: you can just make a helper method somewhere local in FhirIO, like:
   ```
   private HealthcareIOError<String> getStringHealthcareIOError(HealthcareIOError<FhirBundleWithMetadata> in) {
     return new HealthcareIOError<>(
               input.getDataResource().getBundle(),
               input.getErrorMessage(),
               input.getStackTrace(),
               input.getObservedTime(),
               input.getStatusCode()));
   }
   ```
   and apply MapElements to the Results.getFailedBodies() method in-line (like some of the other results methods)



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1524,6 +1551,85 @@ private int parseBundleStatus(String status) {
     }
   }
 
+  /**
+   * ExecuteBundlesResult contains both successfully executed bundles and information help debugging
+   * failed executions (eg metadata & error msgs).
+   */
+  public static class ExecuteBundlesResult extends Write.AbstractResult {
+    private final Pipeline pipeline;
+    private final PCollection<FhirBundleWithMetadata> successfulBundles;
+    private final PCollection<HealthcareIOError<FhirBundleWithMetadata>> failedBundles;
+
+    private ExecuteBundlesResult(
+        Pipeline pipeline,
+        PCollection<FhirBundleWithMetadata> successfulBundles,
+        PCollection<HealthcareIOError<FhirBundleWithMetadata>> failedBundles) {
+      this.pipeline = pipeline;
+      this.successfulBundles = successfulBundles;
+      this.failedBundles = failedBundles;
+    }
+
+    /**
+     * Entry point for the ExecuteBundlesResult, storing the successful and failed bundles and their
+     * metadata.
+     */
+    public static ExecuteBundlesResult in(
+        Pipeline pipeline,
+        PCollection<FhirBundleWithMetadata> successfulBundles,
+        PCollection<HealthcareIOError<FhirBundleWithMetadata>> failedBundles) {
+      return new ExecuteBundlesResult(pipeline, successfulBundles, failedBundles);
+    }
+
+    /** Gets successful FhirBundleWithMetadata from execute bundles operation. */
+    public PCollection<FhirBundleWithMetadata> getSuccessfulBundlesWithMetadata() {
+      return this.successfulBundles;
+    }
+
+    /**
+     * Gets failed FhirBundleWithMetadata with metadata wrapped inside HealthcareIOError. The bundle
+     * field could be null.
+     */
+    public PCollection<HealthcareIOError<FhirBundleWithMetadata>> getFailedBundles() {
+      return this.failedBundles;
+    }
+
+    @Override

Review Comment:
   nit: order the methods to group success/fails, eg: 
   
   getSuccessfulBodies()
   getSuccessfulBundlesWithMetadata()
   getFailedBodies()
   getFailedBundles()
   
   makes it easier to see the difference between methods.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1146248768

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem merged pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem merged PR #17741:
URL: https://github.com/apache/beam/pull/17741


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r884300072


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1492,20 +1513,23 @@ private void parseResponse(ProcessContext context, String inputBody, HttpBody re
             // 20X's are successes, otherwise failure.
             if (statusCode / 100 == 2) {
               success++;
-              context.output(Write.SUCCESSFUL_BODY, entry.toString());
+              context.output(
+                  SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), entry.toString()));
             } else {
               fail++;
               context.output(
-                  Write.FAILED_BODY,
+                  FAILED_BUNDLES,
                   HealthcareIOError.of(
-                      inputBody, HealthcareHttpException.of(statusCode, entry.toString())));
+                      FhirBundleResponse.of(context.element()),
+                      HealthcareHttpException.of(statusCode, entry.toString())));

Review Comment:
   done.



##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIOWriteIT.java:
##########
@@ -121,7 +121,7 @@ public void testFhirIO_ExecuteBundle() throws IOException {
   @Test
   public void testFhirIO_ExecuteBundle_parseResponse() {

Review Comment:
   Done.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883245201


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7

Review Comment:
   Done.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7
+ * message path) in JSON format to be executed on any FHIR store. *
+ */
+@DefaultCoder(FhirBundleWithMetadataCoder.class)
+@AutoValue
+public abstract class FhirBundleWithMetadata {
+
+  static Builder builder() {
+    return new AutoValue_FhirBundleWithMetadata.Builder();
+  }
+
+  /**
+   * String representing the source of the Bundle to be written. Used to pass source data through
+   * the ExecuteBundles PTransform.
+   */
+  public abstract String getMetadata();

Review Comment:
   Done.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883954504


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleResponse.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+
+@AutoValue
+public abstract class FhirBundleResponse {
+
+  static FhirBundleResponse.Builder builder() {
+    return new AutoValue_FhirBundleResponse.Builder();
+  }
+
+  /**
+   * String representing the metadata of the Bundle to be written. Used to pass metadata through the

Review Comment:
   done



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1349,7 +1376,15 @@ public enum ContentStructure {
   }
 
   /** The type Execute bundles. */
-  public static class ExecuteBundles extends Write {
+  public static class ExecuteBundles
+      extends PTransform<PCollection<FhirBundleParameter>, ExecuteBundlesResult> {
+
+    /** The TupleTag used for bundles that were executed successfully. */
+    public static final TupleTag<FhirBundleResponse> SUCCESSFUL_BUNDLES = new TupleTag<>();
+
+    /** The TupleTag used for bundles that failed to be executed for any reason. */
+    public static final TupleTag<HealthcareIOError<FhirBundleResponse>> FAILED_BUNDLES =

Review Comment:
   Done



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883953961


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleResponse.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+
+@AutoValue
+public abstract class FhirBundleResponse {
+
+  static FhirBundleResponse.Builder builder() {
+    return new AutoValue_FhirBundleResponse.Builder();
+  }
+
+  /**
+   * String representing the metadata of the Bundle to be written. Used to pass metadata through the
+   * ExecuteBundles PTransform.
+   */
+  public abstract FhirBundleParameter getFhirBundleParameter();
+
+  /** FHIR R4 bundle resource object as a string. */

Review Comment:
   done



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1146217741

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] msbukal commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
msbukal commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883859096


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1349,7 +1376,15 @@ public enum ContentStructure {
   }
 
   /** The type Execute bundles. */
-  public static class ExecuteBundles extends Write {
+  public static class ExecuteBundles
+      extends PTransform<PCollection<FhirBundleParameter>, ExecuteBundlesResult> {
+
+    /** The TupleTag used for bundles that were executed successfully. */
+    public static final TupleTag<FhirBundleResponse> SUCCESSFUL_BUNDLES = new TupleTag<>();
+
+    /** The TupleTag used for bundles that failed to be executed for any reason. */
+    public static final TupleTag<HealthcareIOError<FhirBundleResponse>> FAILED_BUNDLES =

Review Comment:
   I'd keep the HealthcareIOError as `HealthcareIOError<FhirBundleParameter>` since you aren't adding a response to the FhirBundleResponse, so a user might be confused why the response field is empty.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r881698988


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. Hl7 messageId) in JSON

Review Comment:
   Done



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/GetStringHealthcareIOErrorFn.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import org.apache.beam.sdk.transforms.DoFn;
+
+/**
+ * ParDo function that transforms a HealthcareIOError for a FhirExecuteBundleParameter to an error

Review Comment:
   Done



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] macksclark commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
macksclark commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r880788382


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -17,6 +17,8 @@
  */
 package org.apache.beam.sdk.io.gcp.healthcare;
 
+import static org.apache.beam.sdk.io.gcp.healthcare.FhirIO.ExecuteBundles.FAILED_BUNDLES;

Review Comment:
   This looks like we're importing from the current file which I feel like shouldn't be necessary. Can we define these in an outer class so we have broader access to them, or make them not private?



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. Hl7 messageId) in JSON
+ * format to be executed on the intermediate FHIR store. *

Review Comment:
   can you scrub all instances of "intermediate/final fhir store" please and just generalize to any FHIR store



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. Hl7 messageId) in JSON

Review Comment:
   can you change the example to: `eg. source ID like HL7 message path` 
   
   Also the "L" in HL7 is capital



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -58,6 +60,7 @@
 import org.apache.beam.sdk.io.fs.ResolveOptions.StandardResolveOptions;
 import org.apache.beam.sdk.io.fs.ResourceId;
 import org.apache.beam.sdk.io.gcp.healthcare.FhirIO.Import.ContentStructure;
+import org.apache.beam.sdk.io.gcp.healthcare.FhirIO.Write.AbstractResult;

Review Comment:
   same comment here this is a weird pattern



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/GetStringHealthcareIOErrorFn.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import org.apache.beam.sdk.transforms.DoFn;
+
+/**
+ * ParDo function that transforms a HealthcareIOError for a FhirExecuteBundleParameter to an error
+ * with the body (string) as the data resource, for backwards compatibility.
+ */
+public class GetStringHealthcareIOErrorFn

Review Comment:
   optional: should we just define this as a subclass within the FhirIO Result class? 



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/GetStringHealthcareIOErrorFn.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import org.apache.beam.sdk.transforms.DoFn;
+
+/**
+ * ParDo function that transforms a HealthcareIOError for a FhirExecuteBundleParameter to an error

Review Comment:
   This should read FhirExecuteBundleWithMetadata right? 



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1140525065

   Run Java PostCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1140541590

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] asf-ci commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1146177154

   Can one of the admins verify this patch?


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883954199


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleResponse.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+
+@AutoValue
+public abstract class FhirBundleResponse {
+
+  static FhirBundleResponse.Builder builder() {
+    return new AutoValue_FhirBundleResponse.Builder();
+  }
+
+  /**
+   * String representing the metadata of the Bundle to be written. Used to pass metadata through the
+   * ExecuteBundles PTransform.
+   */
+  public abstract FhirBundleParameter getFhirBundleParameter();
+
+  /** FHIR R4 bundle resource object as a string. */
+  public abstract String getResponse();
+
+  public static FhirBundleResponse of(
+      FhirBundleParameter fhirBundleParameter, @Nullable String response) {
+    return FhirBundleResponse.builder()
+        .setFhirBundleParameter(fhirBundleParameter)
+        .setResponse(Objects.toString(response, ""))
+        .build();
+  }
+
+  public static FhirBundleResponse of(FhirBundleParameter fhirBundleParameter) {

Review Comment:
   Removed it.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1145031788

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1140496185

   Run Java PostCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r881697470


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -58,6 +60,7 @@
 import org.apache.beam.sdk.io.fs.ResolveOptions.StandardResolveOptions;
 import org.apache.beam.sdk.io.fs.ResourceId;
 import org.apache.beam.sdk.io.gcp.healthcare.FhirIO.Import.ContentStructure;
+import org.apache.beam.sdk.io.gcp.healthcare.FhirIO.Write.AbstractResult;

Review Comment:
   Done



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] msbukal commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
msbukal commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883823754


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleResponse.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+
+@AutoValue
+public abstract class FhirBundleResponse {
+
+  static FhirBundleResponse.Builder builder() {
+    return new AutoValue_FhirBundleResponse.Builder();
+  }
+
+  /**
+   * String representing the metadata of the Bundle to be written. Used to pass metadata through the
+   * ExecuteBundles PTransform.
+   */
+  public abstract FhirBundleParameter getFhirBundleParameter();
+
+  /** FHIR R4 bundle resource object as a string. */
+  public abstract String getResponse();
+
+  public static FhirBundleResponse of(
+      FhirBundleParameter fhirBundleParameter, @Nullable String response) {
+    return FhirBundleResponse.builder()
+        .setFhirBundleParameter(fhirBundleParameter)
+        .setResponse(Objects.toString(response, ""))
+        .build();
+  }
+
+  public static FhirBundleResponse of(FhirBundleParameter fhirBundleParameter) {

Review Comment:
   Do we need this constructor?



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1349,7 +1376,15 @@ public enum ContentStructure {
   }
 
   /** The type Execute bundles. */
-  public static class ExecuteBundles extends Write {
+  public static class ExecuteBundles
+      extends PTransform<PCollection<FhirBundleParameter>, ExecuteBundlesResult> {
+
+    /** The TupleTag used for bundles that were executed successfully. */
+    public static final TupleTag<FhirBundleResponse> SUCCESSFUL_BUNDLES = new TupleTag<>();
+
+    /** The TupleTag used for bundles that failed to be executed for any reason. */
+    public static final TupleTag<HealthcareIOError<FhirBundleResponse>> FAILED_BUNDLES =

Review Comment:
   I'd keep the HealthcareIOError as HealthcareIOError<FhirBundleParameter> since you aren't adding a response to the FhirBundleResponse, so a user might be confused why the response field is empty.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1492,20 +1513,23 @@ private void parseResponse(ProcessContext context, String inputBody, HttpBody re
             // 20X's are successes, otherwise failure.
             if (statusCode / 100 == 2) {
               success++;
-              context.output(Write.SUCCESSFUL_BODY, entry.toString());
+              context.output(
+                  SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), entry.toString()));
             } else {
               fail++;
               context.output(
-                  Write.FAILED_BODY,
+                  FAILED_BUNDLES,
                   HealthcareIOError.of(
-                      inputBody, HealthcareHttpException.of(statusCode, entry.toString())));
+                      FhirBundleResponse.of(context.element()),
+                      HealthcareHttpException.of(statusCode, entry.toString())));
             }
           }
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(success);
           EXECUTE_BUNDLE_RESOURCE_ERRORS.inc(fail);
         } else if (bundleType.equals(BUNDLE_RESPONSE_TYPE_TRANSACTION)) {
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(entries.size());
-          context.output(Write.SUCCESSFUL_BODY, bundle.toString());
+          context.output(
+              SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), bundle.toString()));

Review Comment:
   bundle.toString() -> bundle.getAsString() 
   
   I find getAsString tends to be cleaner, and doesn't include excess quotes.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleResponse.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+
+@AutoValue
+public abstract class FhirBundleResponse {
+
+  static FhirBundleResponse.Builder builder() {
+    return new AutoValue_FhirBundleResponse.Builder();
+  }
+
+  /**
+   * String representing the metadata of the Bundle to be written. Used to pass metadata through the

Review Comment:
   This comment is incorrect, please specify it's the input FhirBundleParameter.



##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIOWriteIT.java:
##########
@@ -121,7 +121,7 @@ public void testFhirIO_ExecuteBundle() throws IOException {
   @Test
   public void testFhirIO_ExecuteBundle_parseResponse() {

Review Comment:
   Can you also add getSuccessfulBundles() calls to this test too, and verify the responses? Just verifying they're not empty is fine.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1492,20 +1513,23 @@ private void parseResponse(ProcessContext context, String inputBody, HttpBody re
             // 20X's are successes, otherwise failure.
             if (statusCode / 100 == 2) {
               success++;
-              context.output(Write.SUCCESSFUL_BODY, entry.toString());
+              context.output(
+                  SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), entry.toString()));
             } else {
               fail++;
               context.output(
-                  Write.FAILED_BODY,
+                  FAILED_BUNDLES,
                   HealthcareIOError.of(
-                      inputBody, HealthcareHttpException.of(statusCode, entry.toString())));
+                      FhirBundleResponse.of(context.element()),
+                      HealthcareHttpException.of(statusCode, entry.toString())));

Review Comment:
   entry.toString() -> entry.getAsString() 
   
   I find getAsString tends to be cleaner, and doesn't include excess quotes.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleResponse.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+
+@AutoValue
+public abstract class FhirBundleResponse {
+
+  static FhirBundleResponse.Builder builder() {
+    return new AutoValue_FhirBundleResponse.Builder();
+  }
+
+  /**
+   * String representing the metadata of the Bundle to be written. Used to pass metadata through the
+   * ExecuteBundles PTransform.
+   */
+  public abstract FhirBundleParameter getFhirBundleParameter();
+
+  /** FHIR R4 bundle resource object as a string. */

Review Comment:
   This comment is also wrong right? it's the HTTP response of the request, not the resource.
   
   Also specify that the value varies depending on BATCH vs TRANSACTION bundles (entry vs whole response).



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1524,6 +1548,101 @@ private int parseBundleStatus(String status) {
     }
   }
 
+  /**
+   * ExecuteBundlesResult contains both successfully executed bundles and information help debugging
+   * failed executions (eg metadata & error msgs).
+   */
+  public static class ExecuteBundlesResult extends Write.AbstractResult {
+    private final Pipeline pipeline;
+    private final PCollection<FhirBundleResponse> successfulBundles;
+    private final PCollection<HealthcareIOError<FhirBundleResponse>> failedBundles;
+
+    private ExecuteBundlesResult(
+        Pipeline pipeline,
+        PCollection<FhirBundleResponse> successfulBundles,
+        PCollection<HealthcareIOError<FhirBundleResponse>> failedBundles) {
+      this.pipeline = pipeline;
+      this.successfulBundles = successfulBundles;
+      this.failedBundles = failedBundles;
+    }
+
+    /**
+     * Entry point for the ExecuteBundlesResult, storing the successful and failed bundles and their
+     * metadata.
+     */
+    public static ExecuteBundlesResult in(
+        Pipeline pipeline,
+        PCollection<FhirBundleResponse> successfulBundles,
+        PCollection<HealthcareIOError<FhirBundleResponse>> failedBundles) {
+      return new ExecuteBundlesResult(pipeline, successfulBundles, failedBundles);
+    }
+
+    @Override
+    public PCollection<String> getSuccessfulBodies() {
+      return this.successfulBundles.apply(
+          MapElements.into(TypeDescriptors.strings())
+              .via(bundleResponse -> bundleResponse.getFhirBundleParameter().getBundle()));

Review Comment:
   Apply string coder at the end here.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883955069


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1524,6 +1548,101 @@ private int parseBundleStatus(String status) {
     }
   }
 
+  /**
+   * ExecuteBundlesResult contains both successfully executed bundles and information help debugging
+   * failed executions (eg metadata & error msgs).
+   */
+  public static class ExecuteBundlesResult extends Write.AbstractResult {
+    private final Pipeline pipeline;
+    private final PCollection<FhirBundleResponse> successfulBundles;
+    private final PCollection<HealthcareIOError<FhirBundleResponse>> failedBundles;
+
+    private ExecuteBundlesResult(
+        Pipeline pipeline,
+        PCollection<FhirBundleResponse> successfulBundles,
+        PCollection<HealthcareIOError<FhirBundleResponse>> failedBundles) {
+      this.pipeline = pipeline;
+      this.successfulBundles = successfulBundles;
+      this.failedBundles = failedBundles;
+    }
+
+    /**
+     * Entry point for the ExecuteBundlesResult, storing the successful and failed bundles and their
+     * metadata.
+     */
+    public static ExecuteBundlesResult in(
+        Pipeline pipeline,
+        PCollection<FhirBundleResponse> successfulBundles,
+        PCollection<HealthcareIOError<FhirBundleResponse>> failedBundles) {
+      return new ExecuteBundlesResult(pipeline, successfulBundles, failedBundles);
+    }
+
+    @Override
+    public PCollection<String> getSuccessfulBodies() {
+      return this.successfulBundles.apply(
+          MapElements.into(TypeDescriptors.strings())
+              .via(bundleResponse -> bundleResponse.getFhirBundleParameter().getBundle()));

Review Comment:
   done



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] asf-ci commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1136042455

   Can one of the admins verify this patch?


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1140549913

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1140654590

   R: @pabloem 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] asf-ci commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1146177133

   Can one of the admins verify this patch?


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r886174503


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleResponseCoder.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.beam.sdk.coders.CustomCoder;
+import org.apache.beam.sdk.coders.NullableCoder;
+import org.apache.beam.sdk.coders.StringUtf8Coder;
+
+public class FhirBundleResponseCoder extends CustomCoder<FhirBundleResponse> {
+  private static final NullableCoder<String> STRING_CODER = NullableCoder.of(StringUtf8Coder.of());
+  private final FhirBundleParameterCoder fhirBundleParameterCoder = FhirBundleParameterCoder.of();
+
+  public static FhirBundleResponseCoder of() {
+    return new FhirBundleResponseCoder();
+  }
+
+  @Override
+  public void encode(FhirBundleResponse value, OutputStream outStream) throws IOException {
+    fhirBundleParameterCoder.encode(value.getFhirBundleParameter(), outStream);
+    STRING_CODER.encode(value.getResponse(), outStream);
+  }
+
+  @Override
+  public FhirBundleResponse decode(InputStream inStream) throws IOException {
+    FhirBundleParameter fhirBundleParameter = fhirBundleParameterCoder.decode(inStream);
+    String response = STRING_CODER.decode(inStream);
+    return FhirBundleResponse.of(fhirBundleParameter, response);

Review Comment:
   same, you can use DefaultSchema to avoid writing the coder: https://stackoverflow.com/questions/62546191/how-do-i-use-an-autovalue-data-type-for-my-pcollection-in-apache-beam



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleParameterCoder.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.beam.sdk.coders.CustomCoder;
+import org.apache.beam.sdk.coders.NullableCoder;
+import org.apache.beam.sdk.coders.StringUtf8Coder;
+
+/** Coder for {@link FhirBundleParameter}. */
+public class FhirBundleParameterCoder extends CustomCoder<FhirBundleParameter> {
+  private static final NullableCoder<String> STRING_CODER = NullableCoder.of(StringUtf8Coder.of());
+
+  public static FhirBundleParameterCoder of() {
+    return new FhirBundleParameterCoder();
+  }
+
+  @Override
+  public void encode(FhirBundleParameter value, OutputStream outStream) throws IOException {
+    STRING_CODER.encode(value.getMetadata(), outStream);
+    STRING_CODER.encode(value.getBundle(), outStream);
+  }
+
+  @Override
+  public FhirBundleParameter decode(InputStream inStream) throws IOException {
+    String metadata = STRING_CODER.decode(inStream);
+    String bundle = STRING_CODER.decode(inStream);
+    return FhirBundleParameter.of(metadata, bundle);
+  }
+}

Review Comment:
   you don't need to define a new coder for this. I would suggest you instead use `DefaultSchema(AutoValueSchema.class)` to encode your bundle parameters. See this SO question: https://stackoverflow.com/questions/62546191/how-do-i-use-an-autovalue-data-type-for-my-pcollection-in-apache-beam



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r881697106


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. Hl7 messageId) in JSON
+ * format to be executed on the intermediate FHIR store. *

Review Comment:
   Done



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -17,6 +17,8 @@
  */
 package org.apache.beam.sdk.io.gcp.healthcare;
 
+import static org.apache.beam.sdk.io.gcp.healthcare.FhirIO.ExecuteBundles.FAILED_BUNDLES;

Review Comment:
   Done



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883245618


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7
+ * message path) in JSON format to be executed on any FHIR store. *

Review Comment:
   Done.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1139248610

   Run Java PostCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r888192448


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleResponseCoder.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.beam.sdk.coders.CustomCoder;
+import org.apache.beam.sdk.coders.NullableCoder;
+import org.apache.beam.sdk.coders.StringUtf8Coder;
+
+public class FhirBundleResponseCoder extends CustomCoder<FhirBundleResponse> {
+  private static final NullableCoder<String> STRING_CODER = NullableCoder.of(StringUtf8Coder.of());
+  private final FhirBundleParameterCoder fhirBundleParameterCoder = FhirBundleParameterCoder.of();
+
+  public static FhirBundleResponseCoder of() {
+    return new FhirBundleResponseCoder();
+  }
+
+  @Override
+  public void encode(FhirBundleResponse value, OutputStream outStream) throws IOException {
+    fhirBundleParameterCoder.encode(value.getFhirBundleParameter(), outStream);
+    STRING_CODER.encode(value.getResponse(), outStream);
+  }
+
+  @Override
+  public FhirBundleResponse decode(InputStream inStream) throws IOException {
+    FhirBundleParameter fhirBundleParameter = fhirBundleParameterCoder.decode(inStream);
+    String response = STRING_CODER.decode(inStream);
+    return FhirBundleResponse.of(fhirBundleParameter, response);

Review Comment:
   done



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleParameterCoder.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.beam.sdk.coders.CustomCoder;
+import org.apache.beam.sdk.coders.NullableCoder;
+import org.apache.beam.sdk.coders.StringUtf8Coder;
+
+/** Coder for {@link FhirBundleParameter}. */
+public class FhirBundleParameterCoder extends CustomCoder<FhirBundleParameter> {
+  private static final NullableCoder<String> STRING_CODER = NullableCoder.of(StringUtf8Coder.of());
+
+  public static FhirBundleParameterCoder of() {
+    return new FhirBundleParameterCoder();
+  }
+
+  @Override
+  public void encode(FhirBundleParameter value, OutputStream outStream) throws IOException {
+    STRING_CODER.encode(value.getMetadata(), outStream);
+    STRING_CODER.encode(value.getBundle(), outStream);
+  }
+
+  @Override
+  public FhirBundleParameter decode(InputStream inStream) throws IOException {
+    String metadata = STRING_CODER.decode(inStream);
+    String bundle = STRING_CODER.decode(inStream);
+    return FhirBundleParameter.of(metadata, bundle);
+  }
+}

Review Comment:
   done



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1145102831

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1146215215

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1146170628

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r884324648


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1492,20 +1513,23 @@ private void parseResponse(ProcessContext context, String inputBody, HttpBody re
             // 20X's are successes, otherwise failure.
             if (statusCode / 100 == 2) {
               success++;
-              context.output(Write.SUCCESSFUL_BODY, entry.toString());
+              context.output(
+                  SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), entry.toString()));
             } else {
               fail++;
               context.output(
-                  Write.FAILED_BODY,
+                  FAILED_BUNDLES,
                   HealthcareIOError.of(
-                      inputBody, HealthcareHttpException.of(statusCode, entry.toString())));
+                      FhirBundleResponse.of(context.element()),
+                      HealthcareHttpException.of(statusCode, entry.toString())));
             }
           }
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(success);
           EXECUTE_BUNDLE_RESOURCE_ERRORS.inc(fail);
         } else if (bundleType.equals(BUNDLE_RESPONSE_TYPE_TRANSACTION)) {
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(entries.size());
-          context.output(Write.SUCCESSFUL_BODY, bundle.toString());
+          context.output(
+              SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), bundle.toString()));

Review Comment:
   I had to revert this change because of the exception. Since bundle is a jsonObject and not a JsonPrimitive, getAsString throws UnsupportedOperation exception.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1140518980

   Run Java PostCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] msbukal commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
msbukal commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r882579750


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7
+ * message path) in JSON format to be executed on any FHIR store. *
+ */
+@DefaultCoder(FhirBundleWithMetadataCoder.class)
+@AutoValue
+public abstract class FhirBundleWithMetadata {
+
+  static Builder builder() {
+    return new AutoValue_FhirBundleWithMetadata.Builder();
+  }
+
+  /**
+   * String representing the source of the Bundle to be written. Used to pass source data through
+   * the ExecuteBundles PTransform.
+   */
+  public abstract String getMetadata();
+
+  /** FHIR R4 bundle resource object as a string. */
+  public abstract String getBundle();
+
+  /** HTTP response from the FHIR store after attempting to write the Bundle method. */
+  public abstract String getResponse();

Review Comment:
   Alternatively, could just return the results as PCollection<String> or PCollection<KV<String, String>> where key = metadata, value = the response (this would follow the same pattern as SearchResources, and not require an additional class.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883245460


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirBundleWithMetadata.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import com.google.auto.value.AutoValue;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.coders.DefaultCoder;
+
+/**
+ * FhirBundleWithMetadata represents a FHIR bundle, with it's metadata (eg. source ID like HL7
+ * message path) in JSON format to be executed on any FHIR store. *
+ */
+@DefaultCoder(FhirBundleWithMetadataCoder.class)
+@AutoValue
+public abstract class FhirBundleWithMetadata {
+
+  static Builder builder() {
+    return new AutoValue_FhirBundleWithMetadata.Builder();
+  }
+
+  /**
+   * String representing the source of the Bundle to be written. Used to pass source data through
+   * the ExecuteBundles PTransform.
+   */
+  public abstract String getMetadata();
+
+  /** FHIR R4 bundle resource object as a string. */
+  public abstract String getBundle();
+
+  /** HTTP response from the FHIR store after attempting to write the Bundle method. */
+  public abstract String getResponse();

Review Comment:
   After discussing with the team, decided to go ahead with 'FhirBundleResponse' class.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1524,6 +1551,85 @@ private int parseBundleStatus(String status) {
     }
   }
 
+  /**
+   * ExecuteBundlesResult contains both successfully executed bundles and information help debugging
+   * failed executions (eg metadata & error msgs).
+   */
+  public static class ExecuteBundlesResult extends Write.AbstractResult {
+    private final Pipeline pipeline;
+    private final PCollection<FhirBundleWithMetadata> successfulBundles;
+    private final PCollection<HealthcareIOError<FhirBundleWithMetadata>> failedBundles;
+
+    private ExecuteBundlesResult(
+        Pipeline pipeline,
+        PCollection<FhirBundleWithMetadata> successfulBundles,
+        PCollection<HealthcareIOError<FhirBundleWithMetadata>> failedBundles) {
+      this.pipeline = pipeline;
+      this.successfulBundles = successfulBundles;
+      this.failedBundles = failedBundles;
+    }
+
+    /**
+     * Entry point for the ExecuteBundlesResult, storing the successful and failed bundles and their
+     * metadata.
+     */
+    public static ExecuteBundlesResult in(
+        Pipeline pipeline,
+        PCollection<FhirBundleWithMetadata> successfulBundles,
+        PCollection<HealthcareIOError<FhirBundleWithMetadata>> failedBundles) {
+      return new ExecuteBundlesResult(pipeline, successfulBundles, failedBundles);
+    }
+
+    /** Gets successful FhirBundleWithMetadata from execute bundles operation. */
+    public PCollection<FhirBundleWithMetadata> getSuccessfulBundlesWithMetadata() {
+      return this.successfulBundles;
+    }
+
+    /**
+     * Gets failed FhirBundleWithMetadata with metadata wrapped inside HealthcareIOError. The bundle
+     * field could be null.
+     */
+    public PCollection<HealthcareIOError<FhirBundleWithMetadata>> getFailedBundles() {
+      return this.failedBundles;
+    }
+
+    @Override

Review Comment:
   done.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883245804


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/GetStringHealthcareIOErrorFn.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.healthcare;
+
+import org.apache.beam.sdk.transforms.DoFn;
+
+/**
+ * ParDo function that transforms a HealthcareIOError for a FhirBundleWithMetadata to an error with
+ * the body (string) as the data resource, for backwards compatibility.
+ */
+public class GetStringHealthcareIOErrorFn

Review Comment:
   Added SimpleFunction.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1146273331

   issue unrelated. merging.


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] asf-ci commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1136042460

   Can one of the admins verify this patch?


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] msbukal commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
msbukal commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r885003204


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1492,20 +1513,23 @@ private void parseResponse(ProcessContext context, String inputBody, HttpBody re
             // 20X's are successes, otherwise failure.
             if (statusCode / 100 == 2) {
               success++;
-              context.output(Write.SUCCESSFUL_BODY, entry.toString());
+              context.output(
+                  SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), entry.toString()));
             } else {
               fail++;
               context.output(
-                  Write.FAILED_BODY,
+                  FAILED_BUNDLES,
                   HealthcareIOError.of(
-                      inputBody, HealthcareHttpException.of(statusCode, entry.toString())));
+                      FhirBundleResponse.of(context.element()),
+                      HealthcareHttpException.of(statusCode, entry.toString())));
             }
           }
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(success);
           EXECUTE_BUNDLE_RESOURCE_ERRORS.inc(fail);
         } else if (bundleType.equals(BUNDLE_RESPONSE_TYPE_TRANSACTION)) {
           EXECUTE_BUNDLE_RESOURCE_SUCCESS.inc(entries.size());
-          context.output(Write.SUCCESSFUL_BODY, bundle.toString());
+          context.output(
+              SUCCESSFUL_BUNDLES, FhirBundleResponse.of(context.element(), bundle.toString()));

Review Comment:
   Oh I see, that's fine.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] msbukal commented on a diff in pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
msbukal commented on code in PR #17741:
URL: https://github.com/apache/beam/pull/17741#discussion_r883859096


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -1349,7 +1376,15 @@ public enum ContentStructure {
   }
 
   /** The type Execute bundles. */
-  public static class ExecuteBundles extends Write {
+  public static class ExecuteBundles
+      extends PTransform<PCollection<FhirBundleParameter>, ExecuteBundlesResult> {
+
+    /** The TupleTag used for bundles that were executed successfully. */
+    public static final TupleTag<FhirBundleResponse> SUCCESSFUL_BUNDLES = new TupleTag<>();
+
+    /** The TupleTag used for bundles that failed to be executed for any reason. */
+    public static final TupleTag<HealthcareIOError<FhirBundleResponse>> FAILED_BUNDLES =

Review Comment:
   I'd keep the `HealthcareIOError<FhirBundleResponse>` as `HealthcareIOError<FhirBundleParameter>` since you aren't adding a response to the FhirBundleResponse, so a user might be confused why the response field is empty.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1137294501

   R: @msbukal 


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1139765023

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] pabloem commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1145197144

   Run Java PreCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] fbeevikm commented on pull request #17741: [BEAM-14504] Add support for including addittional parameters to executebundle method in fhirio.

Posted by GitBox <gi...@apache.org>.
fbeevikm commented on PR #17741:
URL: https://github.com/apache/beam/pull/17741#issuecomment-1145021247

   Run Java PostCommit


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org