[
https://issues.apache.org/jira/browse/BEAM-14504?focusedWorklogId=774152&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-774152
]
ASF GitHub Bot logged work on BEAM-14504:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 24/May/22 17:45
Start Date: 24/May/22 17:45
Worklog Time Spent: 10m
Work Description: 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?
Issue Time Tracking
-------------------
Worklog Id: (was: 774152)
Time Spent: 40m (was: 0.5h)
> Add support for including addittional parameters to executebundle method in
> fhirio.
> -----------------------------------------------------------------------------------
>
> Key: BEAM-14504
> URL: https://issues.apache.org/jira/browse/BEAM-14504
> Project: Beam
> Issue Type: Improvement
> Components: io-java-healthcare
> Reporter: Fathima Mohammed
> Assignee: Fathima Mohammed
> Priority: P2
> Time Spent: 40m
> Remaining Estimate: 0h
>
> 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.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
