arjun4084346 commented on code in PR #3999:
URL: https://github.com/apache/gobblin/pull/3999#discussion_r1680122092


##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/proc/ResumeDagProc.java:
##########
@@ -90,7 +90,7 @@ protected void act(DagManagementStateStore 
dagManagementStateStore, Optional<Dag
     dagManagementStateStore.checkpointDag(failedDag.get());
 
     // if it fails here, it will check point the failed dag in the (running) 
dag store again, which is idempotent
-    dagManagementStateStore.deleteFailedDag(failedDag.get());
+    dagManagementStateStore.deleteFailedDag(getDagId());

Review Comment:
   I think we should expect this kind of symmetry. It is not like a rest 
resource, where we do `get resource` and `delete resource`.
   It is a table with <dagId, dag>. Consider it like a `Map<K, V>` with auto 
incrementing key, we do not call `delete<V>` just because we do `add(V)`.
   There are many other examples like, `Stack` `Queue` API. 
   Basically, I believe, a `key` (dagId/dagNodeId) is the best way to choose a 
resource/row/dag **in this case**, not the dag.



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