Conversation
Relates to elastic#29827
|
Pinging @elastic/es-core-features |
|
|
||
| static Request search(final SearchRequest request) throws IOException { | ||
| if (request.types().length > 0) { | ||
| /* |
There was a problem hiding this comment.
The more I look at this the less I like it. I think the other option is to build a HLRC side request object that uses the server-side agg builders. I'm not sure if that is worth it, but it would be a cleaner API. It wouldn't be exactly the API we want in the end, but it'd be better than this.
There was a problem hiding this comment.
@hub-cap pointed me to HLRC's CountRequest which is pretty similar to what I'm describing. Different because instead of rendering a SearchSourceBuilder it we'd just do aggregations and queries and stuff like that.
There was a problem hiding this comment.
I understand the concern but I find it easier if the request object is the same since you can directly use it for rollup or regular search seamlessly.
There was a problem hiding this comment.
I see that argument too!
I think in an ideal world the request for search and the request for rollup search would implement the same interface or have a common superclass....
There was a problem hiding this comment.
@hub-cap, what do you think about merging this like it stands now?
There was a problem hiding this comment.
can we just placeholder the client side object and test the validation there instead? just extend the existing request? if not then im ok merging as is.
There was a problem hiding this comment.
I can't extend SearchRequest because it is final. I'll merge as is then. I think it'd be nice to make the client side search request objects for rollup search at the same time as we make them for search. One day, one day....
| indices. | ||
|
|
||
| Rules for the `index` parameter: | ||
|
|
There was a problem hiding this comment.
This page was rendering badly without this.
| public class RestRollupSearchAction extends BaseRestHandler { | ||
|
|
||
| private static final Set<String> RESPONSE_PARAMS = Collections.singleton(RestSearchAction.TOTAL_HIT_AS_INT_PARAM); | ||
| private static final Set<String> RESPONSE_PARAMS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( |
There was a problem hiding this comment.
This is required to support the HLRC.
| NumericMetricsAggregation.SingleValue maxTemperature = | ||
| response.getAggregations().get("max_temperature"); | ||
| assertThat(maxTemperature.value(), closeTo(49.0, .00001)); | ||
| // end::search-response-response |
There was a problem hiding this comment.
huh. I wonder why that didn't make the build fail.... Let me check
There was a problem hiding this comment.
Ah! I suspect it doesn't fail because the tag checker ignores tailing characters. It probably should only ignore trailing spaces to be honest. But that is a job for another day!
| public class RestRollupSearchAction extends BaseRestHandler { | ||
|
|
||
| private static final Set<String> RESPONSE_PARAMS = Collections.singleton(RestSearchAction.TOTAL_HIT_AS_INT_PARAM); | ||
| private static final Set<String> RESPONSE_PARAMS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( |
|
|
||
| static Request search(final SearchRequest request) throws IOException { | ||
| if (request.types().length > 0) { | ||
| /* |
There was a problem hiding this comment.
I understand the concern but I find it easier if the request object is the same since you can directly use it for rollup or regular search seamlessly.
|
I'm running the full suite of tests on this backport locally so I added the tag. It'll take a few hours. |
|
And I need the full suite because I touched both the rollup code and the client. |
* elastic/6.x: (37 commits) [HLRC] Added support for Follow Stats API (elastic#36253) Exposed engine must have all ops below gcp during rollback (elastic#36159) TEST: Always enable soft-deletes in ShardChangesTests Use delCount of SegmentInfos to calculate numDocs (elastic#36323) Add soft-deletes upgrade tests (elastic#36286) Remove LocalCheckpointTracker#resetCheckpoint (elastic#34667) Option to use endpoints starting with _security (elastic#36379) [CCR] Restructured QA modules (elastic#36404) RestClient: on retry timeout add root exception (elastic#25576) [HLRC] Add support for put privileges API (elastic#35679) HLRC: Add rollup search (elastic#36334) Explicitly recommend to forceMerge before freezing (elastic#36376) Rename internal repository actions to be internal (elastic#36377) Core: Remove parseDefaulting from DateFormatter (elastic#36386) [ML] Prevent stack overflow while copying ML jobs and datafeeds (elastic#36370) Docs: Fix Jackson reference (elastic#36366) [ILM] Fix issue where index may not yet be in 'hot' phase (elastic#35716) Undeprecate /_watcher endpoints (elastic#36269) Docs: Fix typo in bool query (elastic#36350) HLRC: Add delete template API (elastic#36320) ...
|
All backported! |
5 participants
Relates to #29827