You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/08/21 17:27:19 UTC
[GitHub] [beam] trucleduc opened a new pull request #12661: Add export FHIR resources to GCS method for HealthcareApiClient.
trucleduc opened a new pull request #12661:
URL: https://github.com/apache/beam/pull/12661
Add export FHIR resources to GCS method for HealthcareApiClient.
R: @lastomato
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] trucleduc commented on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
trucleduc commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-679271437
@lastomato Just updated the pull request with the ExportGcs IO connector. PTAL.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on a change in pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #12661:
URL: https://github.com/apache/beam/pull/12661#discussion_r477636134
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -220,6 +222,21 @@ public static Import importResources(
return new Import(fhirStore, tempDir, deadLetterDir, contentStructure);
}
+ /**
+ * Export resources to GCS. Intended for use on non-empty FHIR stores
+ *
+ * @param fhirStore the fhir store, in the format:
+ * projects/project_id/locations/location_id/datasets/dataset_id/fhirStores/fhir_store_id
+ * @param exportGcsUriPrefix the destination GCS dir, in the format:
+ * gs://YOUR_BUCKET_NAME/path/to/a/dir
+ * @return the export
+ * @see ExportGcs
+ */
+ public static ExportGcs exportResourcesToGcs(String fhirStore, String exportGcsUriPrefix) {
+ return new ExportGcs(
+ StaticValueProvider.of(fhirStore), StaticValueProvider.of(exportGcsUriPrefix));
+ }
Review comment:
If you support valueproviders internally, you can add another method `exportResourcesToGcs` that receives `ValueProvider<String>` types - this will enable users to use them with themplates
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] trucleduc commented on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
trucleduc commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-682511200
Thanks Pablo. I'll add the comment on top of FhirIO class in a separate pull request. I plan to add another DeID IO connector then I can put comment for both of them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem merged pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
pabloem merged pull request #12661:
URL: https://github.com/apache/beam/pull/12661
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] trucleduc commented on a change in pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
trucleduc commented on a change in pull request #12661:
URL: https://github.com/apache/beam/pull/12661#discussion_r477650841
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -220,6 +222,21 @@ public static Import importResources(
return new Import(fhirStore, tempDir, deadLetterDir, contentStructure);
}
+ /**
+ * Export resources to GCS. Intended for use on non-empty FHIR stores
+ *
+ * @param fhirStore the fhir store, in the format:
+ * projects/project_id/locations/location_id/datasets/dataset_id/fhirStores/fhir_store_id
+ * @param exportGcsUriPrefix the destination GCS dir, in the format:
+ * gs://YOUR_BUCKET_NAME/path/to/a/dir
+ * @return the export
+ * @see ExportGcs
+ */
+ public static ExportGcs exportResourcesToGcs(String fhirStore, String exportGcsUriPrefix) {
+ return new ExportGcs(
+ StaticValueProvider.of(fhirStore), StaticValueProvider.of(exportGcsUriPrefix));
+ }
Review comment:
Nice. Thanks for catching. We indeed want to use this as a template :-)
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -220,6 +222,21 @@ public static Import importResources(
return new Import(fhirStore, tempDir, deadLetterDir, contentStructure);
}
+ /**
+ * Export resources to GCS. Intended for use on non-empty FHIR stores
+ *
+ * @param fhirStore the fhir store, in the format:
+ * projects/project_id/locations/location_id/datasets/dataset_id/fhirStores/fhir_store_id
+ * @param exportGcsUriPrefix the destination GCS dir, in the format:
+ * gs://YOUR_BUCKET_NAME/path/to/a/dir
+ * @return the export
+ * @see ExportGcs
+ */
+ public static ExportGcs exportResourcesToGcs(String fhirStore, String exportGcsUriPrefix) {
+ return new ExportGcs(
+ StaticValueProvider.of(fhirStore), StaticValueProvider.of(exportGcsUriPrefix));
+ }
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lastomato commented on a change in pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
lastomato commented on a change in pull request #12661:
URL: https://github.com/apache/beam/pull/12661#discussion_r477002138
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -220,6 +222,19 @@ public static Import importResources(
return new Import(fhirStore, tempDir, deadLetterDir, contentStructure);
}
+ /**
+ * Export resources to GCS. Intended for use on non-empty FHIR stores
+ *
+ * @param fhirStore the fhir store
Review comment:
I think it's better to add a bit more details here, e.g. the format of an FHIR store name, and the GCS URI prefix etc.
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -1175,4 +1190,94 @@ public void executeBundles(ProcessContext context) {
}
}
}
+
+ /** Export FHIR resources from a FHIR store to new line delimited json files on GCS. */
+ public static class ExportGcs extends PTransform<PBegin, ExportGcs.Result> {
Review comment:
We might want to add some tests for this. You can find a few examples added by Jacob previously.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] trucleduc commented on a change in pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
trucleduc commented on a change in pull request #12661:
URL: https://github.com/apache/beam/pull/12661#discussion_r477328978
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -1175,4 +1190,94 @@ public void executeBundles(ProcessContext context) {
}
}
}
+
+ /** Export FHIR resources from a FHIR store to new line delimited json files on GCS. */
+ public static class ExportGcs extends PTransform<PBegin, ExportGcs.Result> {
Review comment:
Tests added. But I don't have permission on the testing project to run it. In the meantime, I wrote another script to test on my own project (not in this commit but identical to the test) and it worked 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-682303486
FYI @trucleduc consider the final comment I added : )
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-682223833
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lastomato commented on pull request #12661: Add export FHIR resources to GCS method for HealthcareApiClient.
Posted by GitBox <gi...@apache.org>.
lastomato commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-678419898
Do you mind adding @pabloem as a reviewer as well?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] trucleduc edited a comment on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
trucleduc edited a comment on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-680393763
@pabloem You're right that export FHIR doesn't have many options like import, so I dropped the Options class. Can you look at the latest commit?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-682224392
Thanks. LGTM! I will merge as soon as Java Precommit passes.
As a follow up (or if you feel like doing it in this PR that works too) - I would recommend you consider adding a snippet and documentation at the top of the FhirIO.java file so users have a good idea of how to use this: https://github.com/apache/beam/pull/12661/files#diff-e133729ae76dc88e72692983928b801fL87-L174
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] trucleduc removed a comment on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
trucleduc removed a comment on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-680393763
@pabloem You're right that export FHIR doesn't have many options like import, so I dropped the Options class. Can you look at the latest commit?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-681162605
now your tests run as integration tests: https://ci-beam.apache.org/job/beam_PostCommit_Java_PR/434/testReport/org.apache.beam.sdk.io.gcp.healthcare/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] trucleduc commented on a change in pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
trucleduc commented on a change in pull request #12661:
URL: https://github.com/apache/beam/pull/12661#discussion_r477327991
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -220,6 +230,26 @@ public static Import importResources(
return new Import(fhirStore, tempDir, deadLetterDir, contentStructure);
}
+ /**
+ * Export resources to GCS. Intended for use on non-empty FHIR stores
+ *
+ * @return the export
+ * @see ExportGcs
+ */
+ public static ExportGcs exportResourcesToGcs(ExportGcs.Options exportOptions) {
Review comment:
Export FHIR resources doesn't have many options like import so I moved those config into ExportGcs class.
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -220,6 +222,19 @@ public static Import importResources(
return new Import(fhirStore, tempDir, deadLetterDir, contentStructure);
}
+ /**
+ * Export resources to GCS. Intended for use on non-empty FHIR stores
+ *
+ * @param fhirStore the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] trucleduc commented on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
trucleduc commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-680393763
You're right that export FHIR doesn't have many options like import, so I dropped the Options class. Can you look at the latest commit?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12661:
URL: https://github.com/apache/beam/pull/12661#issuecomment-681119907
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on a change in pull request #12661: Add export FHIR resources to GCS IO Connector
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #12661:
URL: https://github.com/apache/beam/pull/12661#discussion_r476927000
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -220,6 +230,26 @@ public static Import importResources(
return new Import(fhirStore, tempDir, deadLetterDir, contentStructure);
}
+ /**
+ * Export resources to GCS. Intended for use on non-empty FHIR stores
+ *
+ * @return the export
+ * @see ExportGcs
+ */
+ public static ExportGcs exportResourcesToGcs(ExportGcs.Options exportOptions) {
Review comment:
It's a little unorthodox to pass a configuration object to a PTransform rather than configure it directly via `withXYZ` calls.
Why did you choose to define ExportGcs.Options rather than make the ExportGcs class hold its own configuration?
I can think of one benefit - It seems that you can define multiple ExportGCS.Options and pass them down a PTransform to execute multiple exports. Did you have this in mind?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org