LLREST: Override strict deprecation per request #36126
Conversation
Adds the ability for individual requests to override strict deprecation warnings. It should also be useful for folks that want to run with strict deprecation enabled but need to make a few deprecated calls in their application. For those folks it could be part of a migration to strict deprecation. It should also allow us to write much more precise tests than we do now. In particular, it'll let us run all of our tests with strict deprecation enabled and disable it on just the requests that we expect to be deprecated. To start us down that path, this depreates `ESRestTestCase#getStrictDeprecationMode`, pointing contributors to the override. It also removes the yaml test's override of `getStrictDeprecationMode` and replaces it with the migration to prove that we can do it. I thought a fair bit about whether or not this override should be on `Request` or `RequestOptions` but I landed on `RequestOptions` because it seems like something that you'd want to do on a whole class of requests across your application. Which is sort of what `RequestOptions` is for, being immutable with a builder and stuff. Relates to elastic#36044
nik9000
added
>feature
:Clients/Java Low Level REST Client
|
Pinging @elastic/es-core-features |
| * {@link RestClientBuilder#setStrictDeprecationMode strict deprecation} | ||
| * . Null means accept the client's default | ||
| */ | ||
| public void setOverrideStrictDeprecationMode(Boolean overrideStrictDeprecationMode) { |
There was a problem hiding this comment.
Minor comment... I think this name is a bit confusing. My first instinct was to set this true if I wanted to override the client's default strict mode (because I want it to be not-strict). But that's not quite how it works, since this A) overrides and B) sets to the boolean you pass.
Maybe just setStrictDeprecationMode(Boolean) is fine, and it's understood/documented that if you set something it overrides the default which is strict?
There was a problem hiding this comment.
That is the name I started with but I second guessed myself. I'll switch it back.
|
<3 Not sure I'm familiar enough with the client code to give a proper review but I like this functionality a lot :) |
|
Thanks for reviewing @javanna and @polyfractal! I've fixed the name and will try and merge in the morning if the builds pass. |
|
@elasticmachine, run docbldesx |
hub-cap
left a comment
hub-cap
left a comment
There was a problem hiding this comment.
Im not a huge fan of 3 state booleans, but I cant think of a "better" solution if we need all 3 states.
|
Superceded by #36345. |
5 participants
Adds the ability for individual requests to override strict deprecation
warnings. It should also be useful for folks that want to run with strict
deprecation enabled but need to make a few deprecated calls in their
application. For those folks it could be part of a migration to strict
deprecation. It should also allow us to write much more precise tests
than we do now. In particular, it'll let us run all of our tests with
strict deprecation enabled and disable it on just the requests that we
expect to be deprecated.
To start us down that path, this depreates
ESRestTestCase#getStrictDeprecationMode, pointing contributors to theoverride. It also removes the yaml test's override of
getStrictDeprecationModeand replaces it with the migration to provethat we can do it.
I thought a fair bit about whether or not this override should be on
RequestorRequestOptionsbut I landed onRequestOptionsbecauseit seems like something that you'd want to do on a whole class of
requests across your application. Which is sort of what
RequestOptionsis for, being immutable with a builder and stuff.
Relates to #36044