REST : Clear Indices Cache API simplify param parsing #29111
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? |
1 similar comment
|
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? |
| clearIndicesCacheRequest.fieldDataCache(request.paramAsBoolean(entry.getKey(), clearIndicesCacheRequest.fieldDataCache())); | ||
| } else if (Fields.FIELDS.match(entry.getKey(), LoggingDeprecationHandler.INSTANCE)) { | ||
| clearIndicesCacheRequest.fields(request.paramAsStringArray(entry.getKey(), clearIndicesCacheRequest.fields())); | ||
| for (String param : request.params().keySet()) { |
There was a problem hiding this comment.
Recently we've been doing something more like this:
clearIndicesCacheRequest.queryCache(request.paramAsBoolean("query", clearIndicesCacheRequest.queryCache()));
clearIndicesCacheRequest.requestCache(request.paramAsBoolean("request", clearIndicesCacheRequest.requestCache()));
...
Rather than the loop. The whole loop thing is more appropriate for by-hand xcontent parsing then url parsing.
nik9000
left a comment
nik9000
left a comment
There was a problem hiding this comment.
Looks good to me.
@elasticmachine, test this please
| clearIndicesCacheRequest.queryCache(request.paramAsBoolean(QUERY, clearIndicesCacheRequest.queryCache())); | ||
| clearIndicesCacheRequest.requestCache(request.paramAsBoolean(REQUEST, clearIndicesCacheRequest.requestCache())); | ||
| clearIndicesCacheRequest.fieldDataCache(request.paramAsBoolean(FIELDDATA, clearIndicesCacheRequest.fieldDataCache())); | ||
| clearIndicesCacheRequest.fields(request.paramAsStringArray(FIELDS, clearIndicesCacheRequest.fields())); |
There was a problem hiding this comment.
super minor nit: I wonder if we even need the string constants at this point :)
There was a problem hiding this comment.
I don't think we either but I know some folks like them so I certainly don't object to them.
There was a problem hiding this comment.
( for consistency ) I think it would be better to remove them
Thanks!
|
Pinging @elastic/es-core-infra |
|
Thanks for doing this @olcbean! I've got a clean build so I'm merging! |
* master: (476 commits) Fix compilation errors in ML integration tests Small code cleanups and refactorings in persistent tasks (elastic#29109) Update allocation awareness docs (elastic#29116) Configure error file for archive packages (elastic#29129) Configure heap dump path for archive packages (elastic#29130) Client: Add missing test getMinGenerationForSeqNo should acquire read lock (elastic#29126) Backport - Do not renew sync-id PR to 5.6 and 6.3 Client: Wrap SSLHandshakeException in sync calls Fix creating keystore when upgrading (elastic#29121) Align thread pool info to thread pool configuration (elastic#29123) TEST: Adjust translog size assumption in new engine Docs: HighLevelRestClient#multiGet (elastic#29095) Client: Wrap synchronous exceptions (elastic#28919) REST: Clear Indices Cache API simplify param parsing (elastic#29111) Fix typo in ExceptionSerializationTests Remove BWC layer for rejected execution exception Fix EsAbortPolicy to conform to API (elastic#29075) [DOCS] Removed prerelease footnote from upgrade table. Docs: Support triple quotes (elastic#28915) ...
6 participants
Simplify the parsing of the params in Clear Indices Cache API, as a follow up to the removing of the deprecated parameter names.