phet commented on code in PR #3664:
URL: https://github.com/apache/gobblin/pull/3664#discussion_r1147916904
##########
gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowConfigV2ResourceLocalHandler.java:
##########
@@ -154,6 +154,53 @@ private String getErrorMessage(FlowSpec flowSpec) {
}
return "Could not form JSON in FlowConfigV2ResourceLocalHandler";
}
+
+ /**
+ * Update flowConfig locally and trigger all listeners iff @param
triggerListener is set to true
+ */
+ @Override
+ public UpdateResponse updateFlowConfig(FlowId flowId, FlowConfig flowConfig,
boolean triggerListener, long modifiedWatermark) {
Review Comment:
the alternative to overriding here in the `FlowConfigV2ResourceLocalHandler`
would be to modify the impl. in the `FlowConfigResourceLocalHandler` (which is
the super class). I don't know enough about the history (of why we still
retain both versions) to judge what's most appropriate.
situating in the super class would bring this behavior to FlowConfig and
FlowConfigV2 APIs. clearly that would change the response semantics of both,
although it would otherwise remain backwards compatible (e.g. for the request
format). of course this current change evolves the FlowConfigV2 API, so the
question is really whether we wish also to evolve the FlowConfig API's update
as well.
either is fine w/ me, so let me know the preference
--
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]
