Returning successful writes in FhirIO.Write.Result #14034

macksclark · 2021-02-22T17:15:35Z

Adding a getSuccesses() method to the FhirIO.Write.Result - useful for the healthcare API users to know which bodies were successfully written to the store.

I also made the check for TupleTags consistent in FhirIO.Read.Result and FhirIO.Search.Result (using the PCollectionTuple .has() method) since the cast from TupleTagList -> java.util.Collection resulted in a ClassCastException for me.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) 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.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	Dataflow	Samza	Twister2
Go	---	---	---
Java
Python		---	---
XLang		---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---		---	---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

solutions accuracy in logging what was written to the fhir store.

method, casting TupleTagList -> Collection creates an exception for some.

macksclark · 2021-02-22T17:16:16Z

R: @pabloem

pabloem · 2021-02-22T19:34:26Z

Run Java PostCommit

pabloem · 2021-02-23T16:33:08Z

for my edification - I see Bodies are String-typed, right? What is a Body in this case? Sometimes we've used JSON Node types for these elements. Would that be a more appropriate type here?

macksclark · 2021-02-23T18:18:47Z

The body is the entire request body that's being passed to the Healthcare client to be the body of the HTTP request. I guess it could be a Json object here if the content-type header is always json, but I don't know that we should rely on that since we already have it in string format and are interpreting the failedBodies as strings

pabloem · 2021-02-23T18:49:07Z

I see. I wonder if it's worth it to give a more functional object (e.g. https://googleapis.github.io/googleapis/java/all/latest/apidocs/com/google/api/HttpBody.html) - I guess users wanting to use this output will have to parse the bodies?

macksclark · 2021-02-23T19:15:09Z

I see, currently we are just returning exactly what the input was, so I think it's fair to assume that users would know how to parse that. I guess it comes down to: should we be returning the input they provided or the HTTP request we sent to the healthcare server that succeeded? I don't have a strong opinion either way, let me know what you think

pabloem · 2021-02-23T19:35:51Z

Run Java PostCommit

pabloem · 2021-02-23T19:37:52Z

that is reasonable to me. I am just trying to make sure we considered all the possibilities, since we will want to avoid backwards incompatible changes later on.
This LGTM! I'll merge once postcommit tests pass. Thanks!

macksclark · 2021-02-23T19:48:47Z

Thank you!

pabloem · 2021-02-23T23:39:51Z

Run Java PostCommit

pabloem · 2021-02-24T03:34:57Z

Run Java PostCommit

pabloem · 2021-02-24T17:56:28Z

thanks @macksclark

…gger * github/master: (123 commits) [BEAM-11899] Bump commons-pool to 2.8.1 and bump commons-dbcp to 2.8.0, Because there is a library dependency Update pillars.yaml (apache#14142) [BEAM-10632] Checkerframework nullness cleanups (apache#14107) [BEAM-11213] Instantiate SparkListenerApplicationStart in a Spark 3 compatible way Fix typos for excluding testMergingCustomWindowsWithoutCustomWindowTypes Specify the time resolution for TestStreamPayload. [BEAM-10961] Enable strict depdency checking for sdks/java/extensions/euphoria (second attempt) [BEAM-11848] Store Docker images in a variable for consistency. Splitting old Go Precommit and new ULR integration test precommit. Moving runner imports out of ptest. Add the TO_STRING capability to Java and Python [BEAM-11848] Fix Docker images list. jdbc python supported Dataflow runner (apache#13960) Adding a warning to use multi-workers on FnApiRunner Fix legend for Python Directrunner microbenchmarks [BEAM-11740] Estimate PCollection byte size [BEAM-10961] enable strict dependency checking for sdks/java/extensions/zetasketch (apache#14093) Map Dataflow JOB_STATE_CANCELLING to Beam RUNNING state [BEAM-11833] Fix reported watermark after restriction split in UnboundedSourceAsSDFRestrictionTracker [BEAM-10761] add reference to BEAM-11761 [BEAM-10961] enable strict dependency checking for flink/job-server Exclude MapState example integration tests from Dataflow runner v2 suite Remove InvalidWindows from Java SDK, instead track "already merged" bit Fix checkstyle in watermark latency benchmark Fix compile breakage in WindmillStateInternals Improve test, error on ALREADY_MERGED. [BEAM-10961] Strict dependency checking for sdks/java/io/gcp (apache#13791) Initial watermark latency benchmark Attempting improvements on DirectRunner Python dash [BEAM-10961] enable strict dependency checking for sdks/java/extensions/google-cloud-platform-core (apache#14084) Merge pull request apache#13802: [BEAM-1474]. Adding MapState and SetState support for the Dataflow runner Remove some false positives Remove nullness warning suppression [BEAM-11861] Add methods to explicitly provide coder for ParquetIO's Parse and ParseFiles (apache#14078) [BEAM-11531] Use pandas 1.2 for python>=3.7 (apache#14099) [BEAM-10961] add reference to BEAM-11761 [BEAM-10961] add explicit compile for auto_value_annotations in sdks/extensions/ml/build.gradle Attempting improvements on DirectRunner Python dash Recognize JOB_STATE_PENDING from Dataflow and map to RUNNING never run checkerframework on tests Puts more expensive BQ empty table check to the right of the 'and' condition (apache#14094) Use the windowing strategy of the input, not output, PCollection of GBK. Do not stage dataflow worker jar when use runner_v2. [BEAM-11870] Re-raise underlying exception for InvocationTargetException (apache#14098) [BEAM-11778] Create a wrapper for ZetaSQL catalog and refactor accordingly. (apache#13934) [BEAM-9378] Add ignored tests which fail in various ways when querying nested structures (apache#14077) Merge Fn API and runner v2 configurations for DataflowRunner Fix up! formatting Add validate runner test for testing custom merging windows fn without custom window types Revert "Revert "[BEAM-2914] Add portable merging window support to Python. (apache#12995)"" [BEAM-10961] fix stray reordering of lines [BEAM-10961] enable strict dependency checking for sdks/java/extensions/sorter [BEAM-10961] enable strict dependency checking for sdks/java/extensions/sketching [BEAM-10961] enable strict dependency checking for sdks/java/extensions/schemaio-expansion-service [BEAM-10961] enable strict dependency checking for sdks/java/extensions/protobuf [BEAM-10961] enable strict dependency checking for sdks/java/extensions/ml [BEAM-10961] enable strict dependency checking for sdks/java/extensions/kyro [BEAM-10961] enable strict dependency checking for sdks/java/extensions/join-library [BEAM-10961] undo line moves (originally intended for alphabeticization) [BEAM-10961] enable strict dependency checking for sdks/java/extensions/jackson [BEAM-10961] Enable strict dependency checking on sdks/java/extensions/sql (apache#13830) [BEAM-10961] enable strict dependency checking for sdks/java/extensions/euphoria [BEAM-10961] enable strict dependency checking for sdks/java/io/parquet (apache#14062) [BEAM-10961] enable strict dependency checking for sdks/java/io/thrift (apache#14066) Refactor ZetaSqlDialectSpecTest and add some passing tests. (apache#14080) [BEAM-11864] Use objects.equals instead of raw comparison [BEAM-11707] Change WindmillStateCache cache invalidation to be based upon reference invalidation instead of expensive set management. Reduce operations of shared cache by caching per-key object sets locally and flushing as groups to shared cache. Remove byte tracking which could be racy based upon background evictions in favor of just iterating for rendering the status page. This also lets us capture more stats. [BEAM-11730] Reduce context switching overhead for appliance reads by issuing reads directly from calling threads in the case that there is no reads being queued. Fix preview Show string from Dataflow service when job terminates in unrecognized state Log a warning when Dataflow returns an unrecognized state Merge pull request apache#14033 from [BEAM-11408] Integrate Python BigQuery sink with GroupIntoBatches Remove SYNCHRONIZED_PROCESSING_TIME from model proto Remove use of model SYNCHRONIZED_PROCESSING_TIME Merge redundant model feature columns in capability matrix Remove MapReduce runner from capability matrix, because it is on a branch and unreleased Remove JStorm runner from capability matrix, because it is on a branch and unreleased Remove retractions from capability matrix, because they do not exist yet Remove metadata-driven triggers from capability matrix, because they do not exist [BEAM-10937] Add Tour of Beam page (apache#13747) [BEAM-11344] Apply "Become a Committer" changes from Website Revamp (apache#14036) Merge pull request apache#14046 from [BEAM-11791] Adding a microbenchmark for TestStream Returning successful writes in FhirIO.Write.Result (apache#14034) Fixup [BEAM-10961] enable strict dependency checking for sdks/java/io/file-based-io-tests (apache#14052) [BEAM-10961] enable strict dependency checking for sdks/java/io/contextualtextio (apache#14049) [BEAM-10961] enable strict dependency checking for sdks/java/io/kinesis (apache#14058) [BEAM-10961] enable strict dependency checking for sdks/java/io/bigquery-io-perf-tests (apache#14048) [BEAM-10961] enable strict dependency checking for sdks/java/io/elasticsearch (apache#14050) [BEAM-10961] enable strict dependency checking for sdks/java/io/expansion-service (apache#14051) [BEAM-10961] enable strict dependency checking for sdks/java/io/jdbc (apache#14055) [BEAM-10961] enable strict dependency checking for sdks/java/io/jms (apache#14056) [BEAM-10961] enable strict dependency checking for sdks/java/io/kafka (apache#14057) [BEAM-10961] enable strict dependency checking for sdks/java/io/hcatalog (apache#14053) [BEAM-11859] Fixed bug in python S3 IO [BEAM-10114] Fix PerSubscriptionPartitionSdf to not rely on the presence of BundleFinalizer [BEAM-10114] Fix PerSubscriptionPartitionSdf to not rely on the presence of BundleFinalizer [BEAM-10961] fix spacing [BEAM-10961] enable strict dependency checking for sdks/java/io/xml [BEAM-10961] enable strict dependency checking for sdks/java/io/tika ...

tvalentyn · 2021-03-23T19:11:18Z

Gentle reminder to please squash commits before merging when applicable, see: https://beam.apache.org/contribute/committer-guide/#finishing-touches for guidelines and rationale. Thanks!

macksclark added 5 commits February 22, 2021 16:13

Returning successes from FhirIO executeBundles. Needed for healthcare

7a60699

solutions accuracy in logging what was written to the fhir store.

Syncing with milenas change

7f891c3

Undo formatting changes from Google auto-formatter.

ba4a111

Adding the tuple tag check for FhirIO.Write.Result creation.

e4768a4

Updating the contains TupleTag check to use the PCollectionTuple .has()

95201ac

method, casting TupleTagList -> Collection creates an exception for some.

macksclark added 2 commits February 22, 2021 17:49

Fix build

05f6aaf

Running spotless apply

655b29b

pabloem merged commit 3983013 into apache:master Feb 24, 2021

Returning successful writes in FhirIO.Write.Result #14034

Returning successful writes in FhirIO.Write.Result #14034

Uh oh!

Conversation

macksclark commented Feb 22, 2021

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

Uh oh!

macksclark commented Feb 22, 2021

Uh oh!

pabloem commented Feb 22, 2021

Uh oh!

pabloem commented Feb 23, 2021

Uh oh!

macksclark commented Feb 23, 2021

Uh oh!

pabloem commented Feb 23, 2021

Uh oh!

macksclark commented Feb 23, 2021

Uh oh!

pabloem commented Feb 23, 2021

Uh oh!

pabloem commented Feb 23, 2021

Uh oh!

macksclark commented Feb 23, 2021

Uh oh!

pabloem commented Feb 23, 2021

Uh oh!

pabloem commented Feb 24, 2021

Uh oh!

pabloem commented Feb 24, 2021

Uh oh!

tvalentyn commented Mar 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants