pabloem commented on a change in pull request #14233:
URL: https://github.com/apache/beam/pull/14233#discussion_r594632249



##########
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -378,6 +381,29 @@ public static Deidentify deidentify(
     return new Deidentify(sourceFhirStore, destinationFhirStore, deidConfig);
   }
 
+  /**
+   * Increments success and failure counters for an LRO. To be used after the 
LRO has completed.
+   * This function leverages the fact that the LRO metadata is always of the 
format: "counter": {
+   * "success": "1", "failure": "1" }
+   *
+   * @param operation LRO operation object.
+   * @param successCounter the success counter for this operation.
+   * @param failureCounter the failure counter for this operation.
+   */
+  private static void incrementLroCounters(
+      Operation operation, Counter successCounter, Counter failureCounter) {
+    Map<String, Object> opMetadata = operation.getMetadata();
+    if (opMetadata.containsKey(LRO_COUNTER_KEY)) {
+      Map<String, String> counters = (Map<String, String>) 
opMetadata.get(LRO_COUNTER_KEY);

Review comment:
       are we guaranteed that this will always be a map? should we handle any 
potential cast exceptions?

##########
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java
##########
@@ -378,6 +381,29 @@ public static Deidentify deidentify(
     return new Deidentify(sourceFhirStore, destinationFhirStore, deidConfig);
   }
 
+  /**
+   * Increments success and failure counters for an LRO. To be used after the 
LRO has completed.
+   * This function leverages the fact that the LRO metadata is always of the 
format: "counter": {
+   * "success": "1", "failure": "1" }
+   *
+   * @param operation LRO operation object.
+   * @param successCounter the success counter for this operation.
+   * @param failureCounter the failure counter for this operation.
+   */
+  private static void incrementLroCounters(
+      Operation operation, Counter successCounter, Counter failureCounter) {
+    Map<String, Object> opMetadata = operation.getMetadata();
+    if (opMetadata.containsKey(LRO_COUNTER_KEY)) {
+      Map<String, String> counters = (Map<String, String>) 
opMetadata.get(LRO_COUNTER_KEY);
+      if (counters.containsKey(LRO_SUCCESS_KEY)) {
+        successCounter.inc(Long.parseLong(counters.get(LRO_SUCCESS_KEY)));

Review comment:
       maybe we can handle cast / parsing exceptions in a single try/catch 
block?
   
   I understand that this is highly unlikely, but since we don't have a way to 
guarantee this at compilation time, we could be stuck with pipelines failing 
indefinitely if the API were to change in any way in the future and we were to 
forget to update this.




----------------------------------------------------------------
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:
[email protected]


Reply via email to