umustafi commented on code in PR #3544:
URL: https://github.com/apache/gobblin/pull/3544#discussion_r955224404


##########
gobblin-api/src/main/java/org/apache/gobblin/service/ServiceConfigKeys.java:
##########
@@ -26,6 +26,7 @@ public class ServiceConfigKeys {
 
   public static final String GOBBLIN_SERVICE_PREFIX = "gobblin.service.";
   public static final String GOBBLIN_SERVICE_JOB_SCHEDULER_LISTENER_CLASS = 
"org.apache.gobblin.service.modules.scheduler.GobblinServiceJobScheduler";
+  public static final String GOBBLIN_ORCHESTRATOR_LISTENER_CLASS = 
"org.apache.gobblin.service.modules.orchestration.Orchestratotar";

Review Comment:
   typo in the class name `Orchestratotar` instead of `Orchestrator`



##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/core/GobblinServiceGuiceModule.java:
##########
@@ -135,6 +135,9 @@ public void configure(Binder binder) {
     binder.bindConstant()
         .annotatedWith(Names.named(InjectionNames.FLOW_CATALOG_LOCAL_COMMIT))
         .to(serviceConfig.isFlowCatalogLocalCommit());
+    binder.bindConstant()

Review Comment:
   what does binding this config here do?



##########
gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/OrchestratorTest.java:
##########
@@ -152,6 +151,32 @@ private TopologySpec initTopologySpec() {
   }
 
   private FlowSpec initFlowSpec() {
+    Properties properties = new Properties();
+    String flowName = "test_flowName";
+    String flowGroup = "test_flowGroup";
+    properties.put(ConfigurationKeys.FLOW_NAME_KEY, flowName);
+    properties.put(ConfigurationKeys.FLOW_GROUP_KEY, flowGroup);
+    properties.put("job.name", flowName);
+    properties.put("job.group", flowGroup);
+    properties.put("specStore.fs.dir", FLOW_SPEC_STORE_DIR);
+    properties.put("specExecInstance.capabilities", "source:destination");
+    properties.put("job.schedule", "0 0 0 ? * * 2050");
+    ;
+    properties.put("gobblin.flow.sourceIdentifier", "source");
+    properties.put("gobblin.flow.destinationIdentifier", "destination");
+    Config config = ConfigUtils.propertiesToConfig(properties);
+
+    FlowSpec.Builder flowSpecBuilder = null;
+    flowSpecBuilder = 
FlowSpec.builder(computeTopologySpecURI(SPEC_STORE_PARENT_DIR,
+            FLOW_SPEC_GROUP_DIR))
+        .withConfig(config)
+        .withDescription(SPEC_DESCRIPTION)
+        .withVersion(SPEC_VERSION)
+        .withTemplate(URI.create("templateURI"));
+    return flowSpecBuilder.build();
+  }
+
+  private FlowSpec initBadFlowSpec() {
     Properties properties = new Properties();

Review Comment:
   can u add an explanation for why this spec does not compile for clarity?



##########
gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/OrchestratorTest.java:
##########
@@ -152,6 +151,32 @@ private TopologySpec initTopologySpec() {
   }
 
   private FlowSpec initFlowSpec() {

Review Comment:
   where is this function being used? are we asserting somewhere that this flow 
does compile?



-- 
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]

Reply via email to