umustafi commented on code in PR #3533:
URL: https://github.com/apache/gobblin/pull/3533#discussion_r940664321
##########
gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowConfigsV2Resource.java:
##########
@@ -125,9 +132,18 @@ public List<FlowConfig> getFilteredFlows(@Context
PagingContext context,
@Optional @QueryParam("isRunImmediately") Boolean isRunImmediately,
@Optional @QueryParam("owningGroup") String owningGroup,
@Optional @QueryParam("propertyFilter") String propertyFilter) {
- FlowSpecSearchObject flowSpecSearchObject = new FlowSpecSearchObject(null,
flowGroup, flowName,
- templateUri, userToProxy, sourceIdentifier, destinationIdentifier,
schedule, null,
- isRunImmediately, owningGroup, propertyFilter);
+ FlowSpecSearchObject flowSpecSearchObject;
+ if (!context.hasCount() && !context.hasStart()){
+ flowSpecSearchObject = new FlowSpecSearchObject(null, flowGroup,
flowName,
+ templateUri, userToProxy, sourceIdentifier, destinationIdentifier,
schedule, null,
+ isRunImmediately, owningGroup, propertyFilter, -1, -1, null);
+ }
+ else {
+ flowSpecSearchObject = new FlowSpecSearchObject(null, flowGroup,
flowName,
+ templateUri, userToProxy, sourceIdentifier, destinationIdentifier,
schedule, null,
+ isRunImmediately, owningGroup, propertyFilter, context.getStart(),
context.getCount(), null);
Review Comment:
what do the start and count values represent?
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStoreTest.java:
##########
@@ -240,6 +243,40 @@ public void testGetSpecWithTag() throws Exception {
Assert.assertEquals(result.size(), 2);
}
+ @Test (dependsOnMethods = "testGetSpec")
+ public void testGetFilterSpecPaginate() throws Exception {
+ FlowSpecSearchObject flowSpecSearchObject =
FlowSpecSearchObject.builder().count(1).start(0).propertyFilter("filter.this.flow").build();
+ Collection<Spec> specs = this.specStore.getSpecs(flowSpecSearchObject);
+ Assert.assertEquals(specs.size(), 1);
+ Assert.assertTrue(specs.contains(this.flowSpec1));
+ Assert.assertFalse(specs.contains(this.flowSpec2));
+ Assert.assertFalse(specs.contains(this.flowSpec4));
+
+ flowSpecSearchObject =
FlowSpecSearchObject.builder().count(1).start(1).propertyFilter("filter.this.flow").build();
+ specs = this.specStore.getSpecs(flowSpecSearchObject);
+ Assert.assertEquals(specs.size(), 1);
+ Assert.assertFalse(specs.contains(this.flowSpec1));
+ Assert.assertTrue(specs.contains(this.flowSpec2));
+ Assert.assertFalse(specs.contains(this.flowSpec4));
+
+ flowSpecSearchObject =
FlowSpecSearchObject.builder().count(5).start(0).propertyFilter("filter.this.flow").build();
+ specs = this.specStore.getSpecs(flowSpecSearchObject);
Review Comment:
its not very clear to me how the count and start values are changing the
spec catalog size here. Is count the limit to number it adds? Is start the
offset from the start of the flow catalog? It would be helpful to add a comment
here to explain why a certain flow spec should not be contained here. ie: the
start value appears to be changing which flow specs are added in the top two
cases so you should make that clear.
--
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]
