Will-Lo commented on code in PR #3533:
URL: https://github.com/apache/gobblin/pull/3533#discussion_r947145973
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_catalog/FlowCatalog.java:
##########
@@ -312,6 +312,14 @@ public Collection<Spec> getAllSpecs() {
}
}
+ public Collection<Spec> getAllSpecs(int start, int count) {
Review Comment:
Add javadocs
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_catalog/FlowCatalog.java:
##########
@@ -312,6 +312,14 @@ public Collection<Spec> getAllSpecs() {
}
}
+ public Collection<Spec> getAllSpecs(int start, int count) {
+ try {
+ return specStore.getSpecs(start, count);
+ } catch (IOException e) {
+ throw new RuntimeException("Cannot retrieve all specs from Spec stores",
e);
Review Comment:
technicaly the exception here would be, cannot retrieve specs from <start>
to <start + count> from Spec stores
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/FlowSpecSearchObject.java:
##########
@@ -181,7 +190,7 @@ public void completePreparedStatement(PreparedStatement
statement)
}
if (this.getSchedule() != null) {
- statement.setString(++i, this.getModifiedTimestamp());
+ statement.setString(++i, this.getSchedule());
Review Comment:
Good catch :)
##########
gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-client/src/main/java/org/apache/gobblin/service/FlowConfigV2Client.java:
##########
@@ -224,6 +237,25 @@ public Collection<FlowConfig> getFlowConfigs(String
flowGroup, String flowName,
return response.getEntity().getElements();
}
+ /**
+ * Get all {@link FlowConfig}s that matches the provided parameters. All the
parameters are optional.
+ * If a parameter is null, it is ignored. {@see
FlowConfigV2Resource#getFilteredFlows}
+ */
+ public Collection<FlowConfig> getFlowConfigs(String flowGroup, String
flowName, String templateUri, String userToProxy,
+ String sourceIdentifier, String destinationIdentifier, String schedule,
Boolean isRunImmediately, String owningGroup,
+ String propertyFilter, int start, int count) throws
RemoteInvocationException {
+ LOG.debug("getFilteredFlows pagination called");
Review Comment:
Same thing here
##########
gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-client/src/main/java/org/apache/gobblin/service/FlowConfigV2Client.java:
##########
@@ -205,6 +205,19 @@ public Collection<FlowConfig> getAllFlowConfigs() throws
RemoteInvocationExcepti
return response.getEntity().getElements();
}
+ /**
+ * Get all {@link FlowConfig}s
+ * @return all {@link FlowConfig}s within the range of start + count - 1,
inclusive
+ * @throws RemoteInvocationException
+ */
+ public Collection<FlowConfig> getAllFlowConfigs(int start, int count) throws
RemoteInvocationException {
+ LOG.debug("getAllFlowConfigs with pagination called");
Review Comment:
I know it's just a debug log but it's good to get into the habit of adding
context into your logs, for example the flowName, flowGroup, start, count. This
is useful when we need to grep and provides us information to debug off of.
--
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]
