[GitHub] [gobblin] phet commented on a diff in pull request #3665: [GOBBLIN-1804] Merge similar logic between `FlowConfig{,V2}ResourceLocalHandler.update` into single base class impl.

["phet (via GitHub)" <gi...@apache.org>]

phet commented on code in PR #3665:
URL: https://github.com/apache/gobblin/pull/3665#discussion_r1149542166


##########
gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowConfigResourceLocalHandler.java:
##########
@@ -305,4 +322,43 @@ public static FlowSpec createFlowSpecForConfig(FlowConfig flowConfig) {
       throw new FlowConfigLoggedException(HttpStatus.S_400_BAD_REQUEST, "bad URI " + flowConfig.getTemplateUris(), e);
     }
   }
+
+  protected String getErrorMessage(FlowSpec flowSpec) {
+    StringBuilder message = new StringBuilder("Flow was not compiled successfully.");
+    Map<String, ArrayList<String>> allErrors = new HashMap<>();

Review Comment:
   FYI, I replaced `HashTable`, as that's a non-canonical collection (even surprising to see in java 1.8 code).  in fact, I first wondered whether there was a specific reason, perhaps due to json serialization... but AFAICT, the current unit tests exercise this function to thus validate that `HashMap` works just 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.

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org