You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "epugh (via GitHub)" <gi...@apache.org> on 2026/08/28 18:12:01 UTC
[PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
epugh opened a new pull request, #4826:
URL: https://github.com/apache/solr/pull/4826
Document our conventions for the V2 APIs.
I'm trying to move towards a more mechanistic process of verifying the V1 -> V2 or V2 legacy to V2 Jax RS migrations and capture the knowledge.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4826:
URL: https://github.com/apache/solr/pull/4826#issuecomment-5551621949
Okay, gave it another work thorugh.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4826:
URL: https://github.com/apache/solr/pull/4826#discussion_r3926799028
##########
dev-docs/v2-api-conventions.adoc:
##########
@@ -139,3 +139,115 @@ In these cases, developers may:
AddFieldOperation requestBody)
throws Exception;
```
+
+== Response POJOs
+
+Every v2 response body extends `SolrJerseyResponse`, which provides `responseHeader` and `error`.
+Beyond that base, pick the narrowest existing response type that fits before writing a new one:
+
+* `SolrJerseyResponse` - plain success/error response with no extra data. The default for simple mutations.
+* `AsyncJerseyResponse extends SolrJerseyResponse` - adds a `requestId` field. Use this (or a subclass of it) for any API that supports the `async` request-body parameter, since `submitRemoteMessageAndHandleAsync`/`handlePotentiallyAsynchronousTask` populate `requestId` automatically when an async id is present.
+* `SubResponseAccumulatingJerseyResponse extends AsyncJerseyResponse` - adds `successfulSubResponsesByNodeName`, `failedSubResponsesByNodeName`, and `warning`. Use this for Overseer-driven APIs that fan out to multiple nodes/replicas (e.g. `CreateShard`, `DeleteShard`) - `AdminAPIBase.submitRemoteMessageAndHandleResponse` populates these fields from the Overseer's `success`/`failure`/`warning` NamedList entries automatically.
+* `FlexibleSolrJerseyResponse extends SolrJerseyResponse` - adds `@JsonAnyGetter`/`@JsonAnySetter`-backed dynamic *top-level* properties, for APIs whose entire response shape is genuinely open-ended (e.g. `SchemaDesigner`, the `Select` query API). This is different from the dynamic-request-POJO pattern above: it's for responses, and the dynamism applies to the whole top level rather than one field.
+
+If an API returns specific, known additional data beyond these bases (e.g. timing information, computed ranges, a resource's status fields), extend the appropriate base with typed `@JsonProperty` fields rather than reaching for `FlexibleSolrJerseyResponse` - see `SplitShardResponse.timing`, `SplitCoreResponse.ranges`, or `CollectionStatusResponse` for examples.
+Don't settle for a bare `SolrJerseyResponse` if the underlying operation actually produces more than a bare success/error - a v2 JSON caller has no other way to get that data, since (unlike v1) nothing else in the response pipeline will surface it (see the async caveat below for why this matters in practice).
+
+== Async Task Handling
+
+Solr has two distinct, non-interchangeable mechanisms for handling the `async` request parameter, depending on which base class the API extends:
+
+* **Core-level (`CoreAdminAPIBase`)**: `handlePotentiallyAsynchronousTask(response, coreName, taskId, actionName, supplier)`. If `taskId` is null, the supplier runs inline and its result is returned directly. If non-null, the supplier is wrapped in a `CoreAdminAsyncTracker.TaskObject` and submitted to `coreAdminAsyncTracker`, which tracks status for later `REQUESTSTATUS` polling. Because the supplier is a `Supplier<T>`, it cannot throw checked exceptions directly - wrap them in `CoreAdminAPIBase.CoreAdminAPIBaseException` and rethrow, which `handlePotentiallyAsynchronousTask` unwraps back to the original checked exception for the caller.
Review Comment:
humm... I think I made this doc after muching around with tasks... Do we have a lot of apis that have the async property? If so, and tif this is good, then it shold be here...?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4826:
URL: https://github.com/apache/solr/pull/4826#issuecomment-5456304094
if soemthing is not found, it's 404 not 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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #4826:
URL: https://github.com/apache/solr/pull/4826#discussion_r3926161892
##########
dev-docs/v2-api-conventions.adoc:
##########
@@ -139,3 +139,115 @@ In these cases, developers may:
AddFieldOperation requestBody)
throws Exception;
```
+
+== Response POJOs
+
+Every v2 response body extends `SolrJerseyResponse`, which provides `responseHeader` and `error`.
+Beyond that base, pick the narrowest existing response type that fits before writing a new one:
+
+* `SolrJerseyResponse` - plain success/error response with no extra data. The default for simple mutations.
+* `AsyncJerseyResponse extends SolrJerseyResponse` - adds a `requestId` field. Use this (or a subclass of it) for any API that supports the `async` request-body parameter, since `submitRemoteMessageAndHandleAsync`/`handlePotentiallyAsynchronousTask` populate `requestId` automatically when an async id is present.
+* `SubResponseAccumulatingJerseyResponse extends AsyncJerseyResponse` - adds `successfulSubResponsesByNodeName`, `failedSubResponsesByNodeName`, and `warning`. Use this for Overseer-driven APIs that fan out to multiple nodes/replicas (e.g. `CreateShard`, `DeleteShard`) - `AdminAPIBase.submitRemoteMessageAndHandleResponse` populates these fields from the Overseer's `success`/`failure`/`warning` NamedList entries automatically.
+* `FlexibleSolrJerseyResponse extends SolrJerseyResponse` - adds `@JsonAnyGetter`/`@JsonAnySetter`-backed dynamic *top-level* properties, for APIs whose entire response shape is genuinely open-ended (e.g. `SchemaDesigner`, the `Select` query API). This is different from the dynamic-request-POJO pattern above: it's for responses, and the dynamism applies to the whole top level rather than one field.
+
+If an API returns specific, known additional data beyond these bases (e.g. timing information, computed ranges, a resource's status fields), extend the appropriate base with typed `@JsonProperty` fields rather than reaching for `FlexibleSolrJerseyResponse` - see `SplitShardResponse.timing`, `SplitCoreResponse.ranges`, or `CollectionStatusResponse` for examples.
+Don't settle for a bare `SolrJerseyResponse` if the underlying operation actually produces more than a bare success/error - a v2 JSON caller has no other way to get that data, since (unlike v1) nothing else in the response pipeline will surface it (see the async caveat below for why this matters in practice).
+
+== Async Task Handling
+
+Solr has two distinct, non-interchangeable mechanisms for handling the `async` request parameter, depending on which base class the API extends:
+
+* **Core-level (`CoreAdminAPIBase`)**: `handlePotentiallyAsynchronousTask(response, coreName, taskId, actionName, supplier)`. If `taskId` is null, the supplier runs inline and its result is returned directly. If non-null, the supplier is wrapped in a `CoreAdminAsyncTracker.TaskObject` and submitted to `coreAdminAsyncTracker`, which tracks status for later `REQUESTSTATUS` polling. Because the supplier is a `Supplier<T>`, it cannot throw checked exceptions directly - wrap them in `CoreAdminAPIBase.CoreAdminAPIBaseException` and rethrow, which `handlePotentiallyAsynchronousTask` unwraps back to the original checked exception for the caller.
+* **Collection-level (`AdminAPIBase`)**: `submitRemoteMessageAndHandleAsync`/`submitRemoteMessageAndHandleResponse(response, action, remoteMessage, asyncId[, timeoutMs])`. The `asyncId` is baked into the submitted `ZkNodeProps` message and handled by the Overseer's own async-tracking machinery; `response.requestId` is populated automatically when `asyncId` is non-null. A timeout-aware overload exists for APIs (like shard split) that legitimately need longer than `CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT` to complete.
+
+Only wrap the branches of an API that are actually meant to support async execution.
+A synchronous-by-design sub-operation (e.g. a "dry run" or "compute recommendations" branch that happens to share a method with the real mutating operation) should bypass async handling entirely rather than being routed through it - see the `async` caveat under "Relationship Between V1 and V2 Implementations" below for a concrete failure mode this avoids.
+
+== Relationship Between V1 and V2 Implementations
+
+Most v2 APIs have a corresponding legacy v1 API (e.g. `/admin/cores?action=RELOAD` backs `POST /api/cores/coreName/reload`).
Review Comment:
[0] This section seems like it might be better placed in `dev-docs/apis.adoc`. There's a section there where we already go through how to write a JAX-RS API step-by-step. The rationale info here would be perfect as a sub-bullet in that step-by-step IMO.
##########
dev-docs/v2-api-conventions.adoc:
##########
@@ -139,3 +139,115 @@ In these cases, developers may:
AddFieldOperation requestBody)
throws Exception;
```
+
+== Response POJOs
+
+Every v2 response body extends `SolrJerseyResponse`, which provides `responseHeader` and `error`.
Review Comment:
Not quite true, there are exceptions where we have a JAX-RS method return 'StreamingOutput'. But maybe it's just being pedantic to mention that here...
##########
dev-docs/v2-api-conventions.adoc:
##########
@@ -139,3 +139,115 @@ In these cases, developers may:
AddFieldOperation requestBody)
throws Exception;
```
+
+== Response POJOs
+
+Every v2 response body extends `SolrJerseyResponse`, which provides `responseHeader` and `error`.
+Beyond that base, pick the narrowest existing response type that fits before writing a new one:
+
+* `SolrJerseyResponse` - plain success/error response with no extra data. The default for simple mutations.
+* `AsyncJerseyResponse extends SolrJerseyResponse` - adds a `requestId` field. Use this (or a subclass of it) for any API that supports the `async` request-body parameter, since `submitRemoteMessageAndHandleAsync`/`handlePotentiallyAsynchronousTask` populate `requestId` automatically when an async id is present.
+* `SubResponseAccumulatingJerseyResponse extends AsyncJerseyResponse` - adds `successfulSubResponsesByNodeName`, `failedSubResponsesByNodeName`, and `warning`. Use this for Overseer-driven APIs that fan out to multiple nodes/replicas (e.g. `CreateShard`, `DeleteShard`) - `AdminAPIBase.submitRemoteMessageAndHandleResponse` populates these fields from the Overseer's `success`/`failure`/`warning` NamedList entries automatically.
+* `FlexibleSolrJerseyResponse extends SolrJerseyResponse` - adds `@JsonAnyGetter`/`@JsonAnySetter`-backed dynamic *top-level* properties, for APIs whose entire response shape is genuinely open-ended (e.g. `SchemaDesigner`, the `Select` query API). This is different from the dynamic-request-POJO pattern above: it's for responses, and the dynamism applies to the whole top level rather than one field.
+
+If an API returns specific, known additional data beyond these bases (e.g. timing information, computed ranges, a resource's status fields), extend the appropriate base with typed `@JsonProperty` fields rather than reaching for `FlexibleSolrJerseyResponse` - see `SplitShardResponse.timing`, `SplitCoreResponse.ranges`, or `CollectionStatusResponse` for examples.
+Don't settle for a bare `SolrJerseyResponse` if the underlying operation actually produces more than a bare success/error - a v2 JSON caller has no other way to get that data, since (unlike v1) nothing else in the response pipeline will surface it (see the async caveat below for why this matters in practice).
+
+== Async Task Handling
+
+Solr has two distinct, non-interchangeable mechanisms for handling the `async` request parameter, depending on which base class the API extends:
+
+* **Core-level (`CoreAdminAPIBase`)**: `handlePotentiallyAsynchronousTask(response, coreName, taskId, actionName, supplier)`. If `taskId` is null, the supplier runs inline and its result is returned directly. If non-null, the supplier is wrapped in a `CoreAdminAsyncTracker.TaskObject` and submitted to `coreAdminAsyncTracker`, which tracks status for later `REQUESTSTATUS` polling. Because the supplier is a `Supplier<T>`, it cannot throw checked exceptions directly - wrap them in `CoreAdminAPIBase.CoreAdminAPIBaseException` and rethrow, which `handlePotentiallyAsynchronousTask` unwraps back to the original checked exception for the caller.
Review Comment:
[-0] This detail is great, but it feels pretty far from this document's original purpose of covering what our v2 APIs should look like cosmetically and JAX-RS implementation details.
Or to put it a different way: there's lots of detail that might be helpful to a person writing a v2 API. The distinction between the overseer and David's "Distributed Command Processing" stuff. CoreContainer vs. SolrCore API registration. How clusterstate is represented in ZooKeeper. But if we're going to jam all that stuff in here, this doc will become an absolute mess!!
I'm not saying it doesn't deserve to be documented, but it might belong better in a different doc?
##########
dev-docs/v2-api-conventions.adoc:
##########
@@ -139,3 +139,115 @@ In these cases, developers may:
AddFieldOperation requestBody)
throws Exception;
```
+
+== Response POJOs
+
+Every v2 response body extends `SolrJerseyResponse`, which provides `responseHeader` and `error`.
+Beyond that base, pick the narrowest existing response type that fits before writing a new one:
+
+* `SolrJerseyResponse` - plain success/error response with no extra data. The default for simple mutations.
+* `AsyncJerseyResponse extends SolrJerseyResponse` - adds a `requestId` field. Use this (or a subclass of it) for any API that supports the `async` request-body parameter, since `submitRemoteMessageAndHandleAsync`/`handlePotentiallyAsynchronousTask` populate `requestId` automatically when an async id is present.
+* `SubResponseAccumulatingJerseyResponse extends AsyncJerseyResponse` - adds `successfulSubResponsesByNodeName`, `failedSubResponsesByNodeName`, and `warning`. Use this for Overseer-driven APIs that fan out to multiple nodes/replicas (e.g. `CreateShard`, `DeleteShard`) - `AdminAPIBase.submitRemoteMessageAndHandleResponse` populates these fields from the Overseer's `success`/`failure`/`warning` NamedList entries automatically.
+* `FlexibleSolrJerseyResponse extends SolrJerseyResponse` - adds `@JsonAnyGetter`/`@JsonAnySetter`-backed dynamic *top-level* properties, for APIs whose entire response shape is genuinely open-ended (e.g. `SchemaDesigner`, the `Select` query API). This is different from the dynamic-request-POJO pattern above: it's for responses, and the dynamism applies to the whole top level rather than one field.
+
+If an API returns specific, known additional data beyond these bases (e.g. timing information, computed ranges, a resource's status fields), extend the appropriate base with typed `@JsonProperty` fields rather than reaching for `FlexibleSolrJerseyResponse` - see `SplitShardResponse.timing`, `SplitCoreResponse.ranges`, or `CollectionStatusResponse` for examples.
+Don't settle for a bare `SolrJerseyResponse` if the underlying operation actually produces more than a bare success/error - a v2 JSON caller has no other way to get that data, since (unlike v1) nothing else in the response pipeline will surface it (see the async caveat below for why this matters in practice).
+
+== Async Task Handling
+
+Solr has two distinct, non-interchangeable mechanisms for handling the `async` request parameter, depending on which base class the API extends:
+
+* **Core-level (`CoreAdminAPIBase`)**: `handlePotentiallyAsynchronousTask(response, coreName, taskId, actionName, supplier)`. If `taskId` is null, the supplier runs inline and its result is returned directly. If non-null, the supplier is wrapped in a `CoreAdminAsyncTracker.TaskObject` and submitted to `coreAdminAsyncTracker`, which tracks status for later `REQUESTSTATUS` polling. Because the supplier is a `Supplier<T>`, it cannot throw checked exceptions directly - wrap them in `CoreAdminAPIBase.CoreAdminAPIBaseException` and rethrow, which `handlePotentiallyAsynchronousTask` unwraps back to the original checked exception for the caller.
+* **Collection-level (`AdminAPIBase`)**: `submitRemoteMessageAndHandleAsync`/`submitRemoteMessageAndHandleResponse(response, action, remoteMessage, asyncId[, timeoutMs])`. The `asyncId` is baked into the submitted `ZkNodeProps` message and handled by the Overseer's own async-tracking machinery; `response.requestId` is populated automatically when `asyncId` is non-null. A timeout-aware overload exists for APIs (like shard split) that legitimately need longer than `CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT` to complete.
+
+Only wrap the branches of an API that are actually meant to support async execution.
+A synchronous-by-design sub-operation (e.g. a "dry run" or "compute recommendations" branch that happens to share a method with the real mutating operation) should bypass async handling entirely rather than being routed through it - see the `async` caveat under "Relationship Between V1 and V2 Implementations" below for a concrete failure mode this avoids.
+
+== Relationship Between V1 and V2 Implementations
+
+Most v2 APIs have a corresponding legacy v1 API (e.g. `/admin/cores?action=RELOAD` backs `POST /api/cores/coreName/reload`).
+Where both exist, **the actual business logic should live in the v2 implementation class, and the v1 handler should delegate to it** - not the other way around.
+This convention exists for a few reasons:
+
+1. It makes it easier to delete the v1 code down the road.
+2. It prevents the v2 code from "falling behind" or being forgotten when query params or API functionality changes, since v1 traffic continuously exercises the same v2 code path.
+3. It gives the v2 endpoint test coverage "for free" - most of Solr's existing test suite targets v1 APIs, so having v1 call v2 gives confidence that the v2 endpoint works correctly even in the absence of direct v2-specific tests.
+
+How this delegation is implemented differs depending on whether the API executes synchronously or is processed by the Overseer.
+
+=== Synchronous (core-level) APIs
Review Comment:
There's a big difference between "core-level" APIs and CoreAdminAPIBase.
"core-level" is used most often to indicate those APIs that are registered on individual SolrCore objects, e.g. "/select", "/update", /admin/ping.
CoreAdminAPIBase is the set of APIs historically offered at `/admin/cores` and that are registered on the CoreContainer.
----
Similarly, down in L199, there's a big difference between "overseer-driven" and "collection-level" APIs. Many collection-level APIs can be run without the overseer (and won't be very soon, if David's push is successful)
----
That's all to say:
1. Since this is user-facing documentation, it's probably worth being a little clearer in our terminology.
2. There's a lot of stuff here that's specific to v2 coverage of CollectionsHandler and CoreAdminHandler. That stuff isn't negligible by any stretch of the imagination, but it's only a fraction of the total v2 API surface. Maybe it's worth moving these docs to a different file that's collectionshandler/coreadminhandler specific so that it doesn't drown out the smaller set of docs that are going to be much more relevant the average "v2 API author"
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4826:
URL: https://github.com/apache/solr/pull/4826#issuecomment-5456301188
Do we need to document plural versus singular? /cluster/package was the plural, and I think it should be /cluster/packages or /packages for example.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4826:
URL: https://github.com/apache/solr/pull/4826#issuecomment-5456100180
@iprithv @jaykay12 since you guys did some recently, what do you think is missing? WOuld love to know, feel free to comment here or edit the doc!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4826:
URL: https://github.com/apache/solr/pull/4826#discussion_r3926804101
##########
dev-docs/v2-api-conventions.adoc:
##########
@@ -139,3 +139,115 @@ In these cases, developers may:
AddFieldOperation requestBody)
throws Exception;
```
+
+== Response POJOs
+
+Every v2 response body extends `SolrJerseyResponse`, which provides `responseHeader` and `error`.
+Beyond that base, pick the narrowest existing response type that fits before writing a new one:
+
+* `SolrJerseyResponse` - plain success/error response with no extra data. The default for simple mutations.
+* `AsyncJerseyResponse extends SolrJerseyResponse` - adds a `requestId` field. Use this (or a subclass of it) for any API that supports the `async` request-body parameter, since `submitRemoteMessageAndHandleAsync`/`handlePotentiallyAsynchronousTask` populate `requestId` automatically when an async id is present.
+* `SubResponseAccumulatingJerseyResponse extends AsyncJerseyResponse` - adds `successfulSubResponsesByNodeName`, `failedSubResponsesByNodeName`, and `warning`. Use this for Overseer-driven APIs that fan out to multiple nodes/replicas (e.g. `CreateShard`, `DeleteShard`) - `AdminAPIBase.submitRemoteMessageAndHandleResponse` populates these fields from the Overseer's `success`/`failure`/`warning` NamedList entries automatically.
+* `FlexibleSolrJerseyResponse extends SolrJerseyResponse` - adds `@JsonAnyGetter`/`@JsonAnySetter`-backed dynamic *top-level* properties, for APIs whose entire response shape is genuinely open-ended (e.g. `SchemaDesigner`, the `Select` query API). This is different from the dynamic-request-POJO pattern above: it's for responses, and the dynamism applies to the whole top level rather than one field.
+
+If an API returns specific, known additional data beyond these bases (e.g. timing information, computed ranges, a resource's status fields), extend the appropriate base with typed `@JsonProperty` fields rather than reaching for `FlexibleSolrJerseyResponse` - see `SplitShardResponse.timing`, `SplitCoreResponse.ranges`, or `CollectionStatusResponse` for examples.
+Don't settle for a bare `SolrJerseyResponse` if the underlying operation actually produces more than a bare success/error - a v2 JSON caller has no other way to get that data, since (unlike v1) nothing else in the response pipeline will surface it (see the async caveat below for why this matters in practice).
+
+== Async Task Handling
+
+Solr has two distinct, non-interchangeable mechanisms for handling the `async` request parameter, depending on which base class the API extends:
+
+* **Core-level (`CoreAdminAPIBase`)**: `handlePotentiallyAsynchronousTask(response, coreName, taskId, actionName, supplier)`. If `taskId` is null, the supplier runs inline and its result is returned directly. If non-null, the supplier is wrapped in a `CoreAdminAsyncTracker.TaskObject` and submitted to `coreAdminAsyncTracker`, which tracks status for later `REQUESTSTATUS` polling. Because the supplier is a `Supplier<T>`, it cannot throw checked exceptions directly - wrap them in `CoreAdminAPIBase.CoreAdminAPIBaseException` and rethrow, which `handlePotentiallyAsynchronousTask` unwraps back to the original checked exception for the caller.
+* **Collection-level (`AdminAPIBase`)**: `submitRemoteMessageAndHandleAsync`/`submitRemoteMessageAndHandleResponse(response, action, remoteMessage, asyncId[, timeoutMs])`. The `asyncId` is baked into the submitted `ZkNodeProps` message and handled by the Overseer's own async-tracking machinery; `response.requestId` is populated automatically when `asyncId` is non-null. A timeout-aware overload exists for APIs (like shard split) that legitimately need longer than `CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT` to complete.
+
+Only wrap the branches of an API that are actually meant to support async execution.
+A synchronous-by-design sub-operation (e.g. a "dry run" or "compute recommendations" branch that happens to share a method with the real mutating operation) should bypass async handling entirely rather than being routed through it - see the `async` caveat under "Relationship Between V1 and V2 Implementations" below for a concrete failure mode this avoids.
+
+== Relationship Between V1 and V2 Implementations
+
+Most v2 APIs have a corresponding legacy v1 API (e.g. `/admin/cores?action=RELOAD` backs `POST /api/cores/coreName/reload`).
Review Comment:
okay... Also, I have been using this to feed to Claude when reviewing v2 migration PR,s.. "Please keep this doc in mind"....
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Documented V2 API Conventions [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4826:
URL: https://github.com/apache/solr/pull/4826#issuecomment-5747165306
I will look to merge this early next week.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4826:
URL: https://github.com/apache/solr/pull/4826#issuecomment-5456289026
We may want to document that these V2 APIs are Experimental, so feel free to change HTTP paths, we don't have any requirement to support old V2, but do need to not touch V1 paths. Jsut experienced this in migrationg the Package API.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4826:
URL: https://github.com/apache/solr/pull/4826#discussion_r3926791238
##########
dev-docs/v2-api-conventions.adoc:
##########
@@ -139,3 +139,115 @@ In these cases, developers may:
AddFieldOperation requestBody)
throws Exception;
```
+
+== Response POJOs
+
+Every v2 response body extends `SolrJerseyResponse`, which provides `responseHeader` and `error`.
Review Comment:
nah, let's modfiy it.... Feel free to edit righ there.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #4826:
URL: https://github.com/apache/solr/pull/4826#discussion_r3926809704
##########
dev-docs/v2-api-conventions.adoc:
##########
@@ -139,3 +139,115 @@ In these cases, developers may:
AddFieldOperation requestBody)
throws Exception;
```
+
+== Response POJOs
+
+Every v2 response body extends `SolrJerseyResponse`, which provides `responseHeader` and `error`.
+Beyond that base, pick the narrowest existing response type that fits before writing a new one:
+
+* `SolrJerseyResponse` - plain success/error response with no extra data. The default for simple mutations.
+* `AsyncJerseyResponse extends SolrJerseyResponse` - adds a `requestId` field. Use this (or a subclass of it) for any API that supports the `async` request-body parameter, since `submitRemoteMessageAndHandleAsync`/`handlePotentiallyAsynchronousTask` populate `requestId` automatically when an async id is present.
+* `SubResponseAccumulatingJerseyResponse extends AsyncJerseyResponse` - adds `successfulSubResponsesByNodeName`, `failedSubResponsesByNodeName`, and `warning`. Use this for Overseer-driven APIs that fan out to multiple nodes/replicas (e.g. `CreateShard`, `DeleteShard`) - `AdminAPIBase.submitRemoteMessageAndHandleResponse` populates these fields from the Overseer's `success`/`failure`/`warning` NamedList entries automatically.
+* `FlexibleSolrJerseyResponse extends SolrJerseyResponse` - adds `@JsonAnyGetter`/`@JsonAnySetter`-backed dynamic *top-level* properties, for APIs whose entire response shape is genuinely open-ended (e.g. `SchemaDesigner`, the `Select` query API). This is different from the dynamic-request-POJO pattern above: it's for responses, and the dynamism applies to the whole top level rather than one field.
+
+If an API returns specific, known additional data beyond these bases (e.g. timing information, computed ranges, a resource's status fields), extend the appropriate base with typed `@JsonProperty` fields rather than reaching for `FlexibleSolrJerseyResponse` - see `SplitShardResponse.timing`, `SplitCoreResponse.ranges`, or `CollectionStatusResponse` for examples.
+Don't settle for a bare `SolrJerseyResponse` if the underlying operation actually produces more than a bare success/error - a v2 JSON caller has no other way to get that data, since (unlike v1) nothing else in the response pipeline will surface it (see the async caveat below for why this matters in practice).
+
+== Async Task Handling
+
+Solr has two distinct, non-interchangeable mechanisms for handling the `async` request parameter, depending on which base class the API extends:
+
+* **Core-level (`CoreAdminAPIBase`)**: `handlePotentiallyAsynchronousTask(response, coreName, taskId, actionName, supplier)`. If `taskId` is null, the supplier runs inline and its result is returned directly. If non-null, the supplier is wrapped in a `CoreAdminAsyncTracker.TaskObject` and submitted to `coreAdminAsyncTracker`, which tracks status for later `REQUESTSTATUS` polling. Because the supplier is a `Supplier<T>`, it cannot throw checked exceptions directly - wrap them in `CoreAdminAPIBase.CoreAdminAPIBaseException` and rethrow, which `handlePotentiallyAsynchronousTask` unwraps back to the original checked exception for the caller.
+* **Collection-level (`AdminAPIBase`)**: `submitRemoteMessageAndHandleAsync`/`submitRemoteMessageAndHandleResponse(response, action, remoteMessage, asyncId[, timeoutMs])`. The `asyncId` is baked into the submitted `ZkNodeProps` message and handled by the Overseer's own async-tracking machinery; `response.requestId` is populated automatically when `asyncId` is non-null. A timeout-aware overload exists for APIs (like shard split) that legitimately need longer than `CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT` to complete.
+
+Only wrap the branches of an API that are actually meant to support async execution.
+A synchronous-by-design sub-operation (e.g. a "dry run" or "compute recommendations" branch that happens to share a method with the real mutating operation) should bypass async handling entirely rather than being routed through it - see the `async` caveat under "Relationship Between V1 and V2 Implementations" below for a concrete failure mode this avoids.
+
+== Relationship Between V1 and V2 Implementations
+
+Most v2 APIs have a corresponding legacy v1 API (e.g. `/admin/cores?action=RELOAD` backs `POST /api/cores/coreName/reload`).
+Where both exist, **the actual business logic should live in the v2 implementation class, and the v1 handler should delegate to it** - not the other way around.
+This convention exists for a few reasons:
+
+1. It makes it easier to delete the v1 code down the road.
+2. It prevents the v2 code from "falling behind" or being forgotten when query params or API functionality changes, since v1 traffic continuously exercises the same v2 code path.
+3. It gives the v2 endpoint test coverage "for free" - most of Solr's existing test suite targets v1 APIs, so having v1 call v2 gives confidence that the v2 endpoint works correctly even in the absence of direct v2-specific tests.
+
+How this delegation is implemented differs depending on whether the API executes synchronously or is processed by the Overseer.
+
+=== Synchronous (core-level) APIs
Review Comment:
yeah, so I was actually thinking this was specific to LLMS reviewing the v2 migrations, and might not even stick aroudn when we are all on v2!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] V2 API Conventions. update our convetnions based on last few migrations [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4826:
URL: https://github.com/apache/solr/pull/4826#issuecomment-5456094603
The typos in my message are so you know this isn't AI generated ;-).
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Documented V2 API Conventions [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh merged PR #4826:
URL: https://github.com/apache/solr/pull/4826
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org