Distinguish Negotiated vs Default Rest API Version #98121
Conversation
This commit changes the RestApiVersion parsing code to return an Optional<RestApiVersion> so that the RestRequest can distinguish between reqeust that included an explicit API version, and requests that should use the server's default version. This is needed because "compatible-with=8" will have a specific meaning on serverless that is not the same as the default API behaviour.
|
@elasticmachine update branch |
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
This is a first step towards having API version negotiation in serverless. In Serverless we want:
To do that we need to keep the "compatible-with" parsing code in place, but distinguish be able to tell whether the parameter was specified or not. |
| @@ -62,7 +63,7 @@ public class RestRequest implements ToXContent.Params { | |||
| private final HttpChannel httpChannel; | |||
| private final ParsedMediaType parsedAccept; | |||
| private final ParsedMediaType parsedContentType; | |||
| private final RestApiVersion restApiVersion; | |||
| private final Optional<RestApiVersion> restApiVersion; | |||
There was a problem hiding this comment.
Is there a reason why this is kept as an optional field, and we do a orElse(RestApiVersion.current()); every time? Can RestApiVersion.current() change, or could we resolve this at construction time and avoid the Optional field?
There was a problem hiding this comment.
Technically we can replace it with a nullable field instead of an Optional.
I didn't do that because I appreciate the support that the compiler provides in avoiding accidental access to a potentially null value, but preferences will vary.
Or alternatively 2 fields (RestApiVersion and boolean) that effectively replicate the behaviour of an Optional (but resolve the orElse at construction time)
There was a problem hiding this comment.
I was thinking about the 2 fields, but the difference is indeed minimal and I think a matter of preference. Thanks for looking into this though!
|
@elasticmachine update branch |
tvernum
added
the
auto-merge-without-approval
|
@elasticmachine run elasticsearch-ci/part-1 please 😿 |
|
@elasticmachine update branch |
This commit changes the RestApiVersion parsing code to return an Optional so that the RestRequest can distinguish between reqeust that included an explicit API version, and requests that should use the server's default version.
This is needed because "compatible-with=8" will have a specific meaning on serverless that is not the same as the default API behaviour.