cryptoe commented on code in PR #18416:
URL: https://github.com/apache/druid/pull/18416#discussion_r2288333829
##########
docs/api-reference/supervisor-api.md:
##########
@@ -1827,6 +1827,12 @@ Retrieves an audit history of specs for a single
supervisor.
`GET` `/druid/indexer/v1/supervisor/{supervisorId}/history`
+#### Query parameters
+
+* `count` (optional)
+ * Maximum number of history entries to return. If not specified, returns all
history entries.
Review Comment:
I think we might want to call out the sorting order here. No ?
##########
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResourceTest.java:
##########
@@ -1134,12 +1134,94 @@ public void testSpecGetHistoryWithAuthFailure()
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.absent());
replayAll();
- response = supervisorResource.specGetHistory(request, "id1");
+ response = supervisorResource.specGetHistory(request, "id1", null);
verifyAll();
Assert.assertEquals(503, response.getStatus());
}
+ @Test
+ public void testSpecGetHistoryWithLimit()
+ {
+ List<VersionedSupervisorSpec> versions = ImmutableList.of(
+ new VersionedSupervisorSpec(
+ new TestSupervisorSpec("id1", null,
Collections.singletonList("datasource1")),
+ "v1"
+ ),
+ new VersionedSupervisorSpec(
+ new TestSupervisorSpec("id1", null,
Collections.singletonList("datasource1")),
+ "v2"
+ ),
+ new VersionedSupervisorSpec(
+ new TestSupervisorSpec("id1", null,
Collections.singletonList("datasource1")),
+ "v3"
+ )
+ );
+
+ List<VersionedSupervisorSpec> limitedVersions = ImmutableList.of(
+ new VersionedSupervisorSpec(
+ new TestSupervisorSpec("id1", null,
Collections.singletonList("datasource1")),
+ "v1"
+ ),
+ new VersionedSupervisorSpec(
+ new TestSupervisorSpec("id1", null,
Collections.singletonList("datasource1")),
+ "v2"
+ )
+ );
+
+
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.of(supervisorManager)).times(4);
+ EasyMock.expect(supervisorManager.getSupervisorHistoryForId("id1",
2)).andReturn(limitedVersions).times(1);
+ EasyMock.expect(supervisorManager.getSupervisorHistoryForId("id1",
0)).andReturn(Collections.emptyList()).times(1);
+ EasyMock.expect(supervisorManager.getSupervisorHistoryForId("id1",
-1)).andReturn(versions).times(1);
+ EasyMock.expect(supervisorManager.getSupervisorHistoryForId("id1",
100)).andReturn(versions).times(1);
+ setupMockRequest();
+ replayAll();
+
+ // Test with valid limit
+ Response response = supervisorResource.specGetHistory(request, "id1", 2);
+ Assert.assertEquals(200, response.getStatus());
+ Assert.assertEquals(limitedVersions, response.getEntity());
+
+ // Test with limit=0
+ response = supervisorResource.specGetHistory(request, "id1", 0);
+ Assert.assertEquals(200, response.getStatus());
+ Assert.assertEquals(Collections.emptyList(), response.getEntity());
+
+ // Test with negative limit
+ response = supervisorResource.specGetHistory(request, "id1", -1);
Review Comment:
Should count <=0 be a bad request ie 400 ?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
