Abacn commented on code in PR #28137:
URL: https://github.com/apache/beam/pull/28137#discussion_r1321740668
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/healthcare/DicomIOIT.java:
##########
@@ -19,55 +19,67 @@
import static
org.apache.beam.sdk.io.gcp.healthcare.HL7v2IOTestUtil.HEALTHCARE_DATASET_TEMPLATE;
+import com.google.api.services.healthcare.v1.model.DeidentifyConfig;
import java.io.IOException;
import java.net.URISyntaxException;
-import org.apache.beam.sdk.PipelineResult;
-import org.apache.beam.sdk.testing.PAssert;
+import java.security.SecureRandom;
import org.apache.beam.sdk.testing.TestPipeline;
-import org.apache.beam.sdk.transforms.Create;
import org.junit.After;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
@SuppressWarnings({"nullness", "uninitialized"})
-public class DicomIOReadIT {
- private static final String TEST_FILE_PATH =
"src/test/resources/DICOM/testDicomFile.dcm";
- private static final String TEST_FILE_STUDY_ID = "study_000000000";
+public class DicomIOIT {
Review Comment:
The test has been rewritten. Is there a chance to remove these
SuppressWarnings?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HealthcareApiClient.java:
##########
@@ -248,4 +248,8 @@ FhirStore createFhirStore(String dataset, String name,
String version, String pu
Empty deleteDicomStore(String name) throws IOException;
Empty uploadToDicomStore(String webPath, String filePath) throws
IOException, URISyntaxException;
+
+ Operation deidentifyDicomStore(
Review Comment:
Though existing interface methods are sparsely documented - which is not
good - could you please add some comment to this newly added method?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/DicomIO.java:
##########
@@ -51,8 +64,33 @@
* readMetadataResult.getReadResponse() PCollection<String> failRead =
* readMetadataResult.getFailedReads() }
*/
+
+// @SuppressWarnings({
+// "nullness" // TODO(https://github.com/apache/beam/issues/20497)
+// })
public class DicomIO {
+ static final String BASE_METRIC_PREFIX = "dicomio/";
+ private static final String LRO_COUNTER_KEY = "counter";
+ private static final String LRO_SUCCESS_KEY = "success";
+ private static final String LRO_FAILURE_KEY = "failure";
+ private static final Logger LOG = LoggerFactory.getLogger(DicomIO.class);
+
+ public static Deidentify deidentify(
+ String sourceDicomStore, String destinationDicomStore, DeidentifyConfig
deidConfig) {
+ return new Deidentify(
+ StaticValueProvider.of(sourceDicomStore),
+ StaticValueProvider.of(destinationDicomStore),
+ StaticValueProvider.of(deidConfig));
+ }
+
+ public static Deidentify deidentify(
+ ValueProvider<String> sourceDicomStore,
+ ValueProvider<String> destinationDicomStore,
+ ValueProvider<DeidentifyConfig> deidConfig) {
Review Comment:
DeidentifyConfig is a protobuf generated message. I'm not sure how a
ValueProvider of this (not String or Json) type could be used by downstream
service, e.g. DataflowTemplate. Probably good to confirm with CC: @bvolpato
In general, we would like the exposed method for user faced XXIO class to be
stable in signature
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
