yukuai518 commented on a change in pull request #2567: GOBBLIN-696: Provide an
"explain" option to return a compiled flow wh…
URL: https://github.com/apache/incubator-gobblin/pull/2567#discussion_r265307112
##########
File path:
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_catalog/FlowCatalog.java
##########
@@ -239,7 +243,7 @@ public Spec getSpec(URI uri) throws SpecNotFoundException {
}
}
- public void put(Spec spec, boolean triggerListener) {
+ public String put(Spec spec, boolean triggerListener) {
Review comment:
I don't think String return type is good enough. First we could invoke
multiple listeners instead of just GobblinServiceJobScheduler. Also even if
GobblinServiceJobScheduler is the only listener here, its return type doesn't
have to be a String.
I think we should have a generic return type such as 'AddSpecResponse' and
allow each listener to implement its own "AddSpecResponse", so the method would
now look like "public AddSpecResponse onAddJob(JobSpec addedJob)". In this way
we are not assuming all the listeners has to be in the same return type. Also
you don't have to generalize or change the MutableSpecCatalog's signature
because all the Polymorphism will be handled by AddSpecResponse's
implementation. You can let GobblinServiceJobScheduler returns an
AddSpecResponse which implements a toString() that is equivalent to your
compiled flow?
----------------------------------------------------------------
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]
With regards,
Apache Git Services
