Conversation
Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…ericSolrRequest Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…lr into copilot/migrate-cancel-task-api
| @Operation( | ||
| summary = "Cancel a currently-running task", | ||
| tags = {"tasks"}) | ||
| FlexibleSolrJerseyResponse cancelTask(@QueryParam("queryUUID") String queryUUID) throws Exception; |
There was a problem hiding this comment.
In the Ref Guide https://solr.apache.org/guide/solr/latest/deployment-guide/task-management.html#taskuuid-parameter we refer when listing tasks to taskUUID but here we have queryUUID, which we also refer to in https://solr.apache.org/guide/solr/latest/deployment-guide/task-management.html#cancelling-an-active-cancellable-task, but then also call it a taskUUID! I don't know that we want to get into fixing the APIS as part of the migration, but this one does seem werid. If someone else confirms it, at a minimum we should open a JIRA. But maybe this is targeted enough to fix here? Seems like it should be taskUUID and honestly, maybe just taskID?
| final FlexibleSolrJerseyResponse response = | ||
| request.process(solrTestRule.getSolrClient(DEFAULT_TEST_COLLECTION_NAME)); | ||
| assertNotNull(response); | ||
| assertEquals("not found", response.unknownProperties().get("cancellationResult")); |
There was a problem hiding this comment.
I think the unknownProperties is an aspect of hte FlexibleSolrJerseyResponse
There was a problem hiding this comment.
Pull request overview
This PR migrates the query-cancellation “cancel task” endpoint from Solr’s legacy @EndPoint API wiring to JAX-RS, and updates the integration test to use the generated SolrJ client and a flexible response model that can capture dynamic fields.
Changes:
- Introduces a new JAX-RS
CancelTaskApiinterface undersolr/apiusing@Path,@GET, and@QueryParam. - Refactors core-side wiring so
QueryCancellationHandlerregistersCancelTaskAPIas a Jersey resource (instead of usingAnnotatedApi), andCancelTaskAPIadapts handler output intoFlexibleSolrJerseyResponse. - Adds an integration test that calls the endpoint using the generated
TasksApi.CancelTaskrequest class.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| solr/core/src/test/org/apache/solr/handler/admin/api/CancelTaskAPITest.java | Adds an integration test using the generated TasksApi.CancelTask client and asserts the flexible response captures cancellationResult. |
| solr/core/src/java/org/apache/solr/handler/component/QueryCancellationHandler.java | Switches v2 exposure from AnnotatedApi to Jersey resource registration (getJerseyResources). |
| solr/core/src/java/org/apache/solr/handler/admin/api/CancelTaskAPI.java | Implements the new JAX-RS interface and adapts handler output into FlexibleSolrJerseyResponse. |
| solr/api/src/java/org/apache/solr/client/api/endpoint/CancelTaskApi.java | Adds the JAX-RS endpoint definition for canceling tasks under the index path prefix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
solr/core/src/java/org/apache/solr/handler/admin/api/CancelTaskAPI.java
Outdated
Show resolved
Hide resolved
solr/api/src/java/org/apache/solr/client/api/endpoint/CancelTaskApi.java
Outdated
Show resolved
Hide resolved
…kAPI.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…kAPI.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…skApi.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…lr into copilot/migrate-cancel-task-api
Migrates
CancelTaskAPIfrom the homegrown@EndPointannotation to standard JAX-RS, and replaces the manualGenericSolrRequestusage in the integration test with the code-generatedTasksApi.CancelTaskSolrJ client.API interface (
solr/api)CancelTaskApiinterface with@Path,@GET,@QueryParamJAX-RS annotations using@StoreApiParametersfor core/collection path supportFlexibleSolrJerseyResponse(instead ofSolrJerseyResponse) so the dynamiccancellationResultfield is captured via@JsonAnySetterwhen deserializingServer implementation (
solr/core)CancelTaskAPIextendsJerseyResource, implementsCancelTaskApi, uses@InjectforSolrCore/SolrQueryRequest/SolrQueryResponseQueryCancellationHandler.handleRequestBody()(logic stays in V1 handler), then copies non-header entries fromsolrQueryResponseinto the returned jersey response — required because Jersey's JSON path serializes the returned object directly, notsolrQueryResponseQueryCancellationHandlernow usesgetJerseyResources()/ emptygetApis()instead ofAnnotatedApiTest
Uses the generated
TasksApi.CancelTaskwithSolrJettyTestRule. The generated class handles V2 URL routing (/api/…vs/solr/…) automatically viagetApiVersion() == V2, eliminating the need to construct a separate V2HttpSolrClient.