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


##########
gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/OrchestratorTest.java:
##########
@@ -244,9 +250,10 @@ public void createTopologySpec() {
     }
     // Make sure TopologyCatalog is empty
     Assert.assertTrue(specs.size() == 0, "Spec store should be empty before 
addition");
-    // Make sure TopologyCatalog Listener is empty
-    Assert.assertTrue(specCompiler.getTopologySpecMap().size() == 0, 
"SpecCompiler should not know about any Topology "
-        + "before addition");
+    Assert.assertTrue(specCompiler.getTopologySpecMap().size() == 1, 
"SpecCompiler should know about any Topology "
+        + " irrespective of what is there in the topology catalog");
+    // Make sure TopologyCatalog empty
+    Assert.assertTrue(this.topologyCatalog.getSize() == 0, "Topology catalog 
should contain 0 Spec before addition");

Review Comment:
   prior to this change, there's an "inconsistent timespan", where the compiler 
doesn't have topo specs initialized.  you suggested this might compromise 
correctness of `DagManagementStateStore`.
   
   after this change, sounds like there will be an "inconsistent timespan", 
where the `TopologyCatalog` may be out of sync w/ the topo specs the compiler 
knows about.  could there be any negative consequence to that?



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