-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Deprecate request_cache for clear-cache #23638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
I just realised that my withPath("_cache") should be withPath("/_cache/clear"), but it doesn't actually make a difference to the test as I am just using the request body to instantiate a ClearIndicesCacheRequest. Should I still be including the withPath() call at all? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I still be including the withPath() call at all?
I'd drop it entirely, yeah.
--- | ||
"clear_cache with request_cache set to false": | ||
- skip: | ||
version: " - 5.2.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This'll be 5.4.0
rather than 5.2.2
, probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably 5.3.99?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I always forget the .99
thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I'll try and merge it this afternoon. I've fried my nice desktop so I'm going to have to rebuild it first....
Sorry, didn't get to this yet. 🔥 💻 🔥 🚒 🚒 🚒 |
In #23638 we renamed `request_cache` to `request` in the `_cache/clear` API. But it is only going to be committed back to 5.x so we can't test with the new name in a mixed version cluster.
In #23638 we renamed `request_cache` to `request` in the `_cache/clear` API. But it is only going to be committed back to 5.x so we can't test with the new name in a mixed version cluster.
I've merged this to master and 5.x. I pushed 2e3ea91 to correct some errors I saw during the backport. CI is a bit upset with this change because the 5.x build hasn't finished yet but it should calm down soon. |
Thanks @StefanGor! |
* master: Docs: Fix language on a few snippets Painless: Fix regex lexer and error messages (elastic#23634) Skip 5.4 bwc test for new name for now Count through the primary in list of strings test Skip testing new name if it isn't known Wait for all shards in list of strings test Deprecate request_cache for clear-cache (elastic#23638)
Fixes issue #22748.
This commit deprecates the use of 'request_cache' to clear the cache. Instead, 'request' should be used.
This is my first contribution, so any feedback is appreciated, particularly around code conventions and the versioning I specified in the test case.