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: [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]
