umustafi commented on code in PR #3700:
URL: https://github.com/apache/gobblin/pull/3700#discussion_r1210851134
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/DagActionStore.java:
##########
@@ -70,21 +73,23 @@ public DagAction(String flowGroup, String flowName, String
flowExecutionId, DagA
* @param flowGroup flow group for the dag action
* @param flowName flow name for the dag action
* @param flowExecutionId flow execution for the dag action
+ * @param dagActionValue the value of the dag action
* @throws IOException
* @return true if we successfully delete one record, return false if the
record does not exist
*/
- boolean deleteDagAction(String flowGroup, String flowName, String
flowExecutionId) throws IOException;
+ boolean deleteDagAction(String flowGroup, String flowName, String
flowExecutionId, DagActionValue dagActionValue) throws IOException;
/***
* Retrieve action value by the flow group, flow name and flow execution id
from the {@link DagActionStore}.
* @param flowGroup flow group for the dag action
* @param flowName flow name for the dag action
* @param flowExecutionId flow execution for the dag action
+ * @param dagActionValue the value of the dag action
* @throws IOException Exception in retrieving the {@link DagAction}.
* @throws SpecNotFoundException If {@link DagAction} being retrieved is not
present in store.
*/
- DagAction getDagAction(String flowGroup, String flowName, String
flowExecutionId) throws IOException, SpecNotFoundException,
-
SQLException;
+ DagAction getDagAction(String flowGroup, String flowName, String
flowExecutionId, DagActionValue dagActionValue)
Review Comment:
You're right actually using `getDagAction` doesn't make sense anymore since
the action is part of the primary key. Instead it may be useful to have
`getDagActions(flow identifiers)` to get all pending actions associated with a
flow right now. We don't have any explicit use case at the moment so I will
remove this method.
Any method now with the store needs all columns that comprise the primary
key, so we can actually pass `DagAction` to any of these functions but looking
at how the functions are used we will end up creating a new `DagAction` object
then pass to the function then unpack those values anyway so I am not certain
that changing the signature is that beneficial unless we care more about
encapsulating the idea that the PK is needed for all of these actions and that
`DagAction` is PK.
--
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]
