Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Partial Results] Move inspector adapter integration into search source #96241

Merged
merged 6 commits into from
Apr 9, 2021
Merged

[Partial Results] Move inspector adapter integration into search source #96241

merged 6 commits into from
Apr 9, 2021

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Apr 5, 2021 •
edited
Loading

Summary

Partially resolves #83652

Release Note

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@lizozom lizozom self-assigned this Apr 5, 2021
@lizozom lizozom added Team:AppServices v7.13.0 v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed release_note:enhancement labels Apr 5, 2021
@lizozom lizozom requested a review from ppisljar April 5, 2021 18:52
@lizozom lizozom mentioned this pull request Apr 5, 2021
Closed
14 tasks
@lizozom lizozom marked this pull request as ready for review April 6, 2021 10:33
@lizozom lizozom requested a review from a team April 6, 2021 10:33
@lizozom lizozom requested a review from a team as a code owner April 6, 2021 10:33
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

ppisljar
ppisljar reviewed Apr 7, 2021
View reviewed changes
src/plugins/data/common/search/expressions/esaggs/request_handler.ts
const response$ = await requestSearchSource.fetch$({
abortSignal,
sessionId: searchSessionId,
requestResponder,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it makes sense here to pass inspectorAdapters.requests in, as we'll need that to construct the other bucket inside searchsource as well. i'm fine with leaving it as is for now however as we plan to move the other bucket code inside searchsource in a followup pr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree and I intend to make that change in the other bucket PR

ppisljar
ppisljar approved these changes Apr 7, 2021
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM

@lizozom lizozom added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Apr 7, 2021
@lizozom lizozom changed the title Move inspector adapter integration into search source [Partial Results] Move inspector adapter integration into search source Apr 7, 2021
@lizozom lizozom mentioned this pull request Apr 7, 2021
Merged
9 tasks
majagrubic
majagrubic approved these changes Apr 8, 2021
View reviewed changes
Copy link
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

search-embeddable changes LGTM 👍

src/plugins/data/common/search/search_source/search_source.ts
async getSearchRequestBody() {
const searchRequest = await this.flatten();
return searchRequest.body;
getSearchRequestBody() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not async any more?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flatten was never async, this is just ceanup

@ppisljar
Copy link
Member

ppisljar commented Apr 8, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@lizozom
Copy link
Contributor Author

lizozom commented Apr 8, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
data 561 562 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
discover 393.9KB 393.5KB -410.0B
maps 2.6MB 2.6MB -444.0B
total -854.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 787.2KB 788.0KB +777.0B
discover 74.7KB 74.4KB -289.0B
total +488.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @lizozom

@lizozom lizozom merged commit b099a0b into elastic:master Apr 9, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Apr 9, 2021
@lizozom @kibanamachine
[Partial Results] Move inspector adapter integration into search sour…
0208c96 
…ce (elastic#96241)

* Move inspector adapter integration into search source

* docs and ts

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine mentioned this pull request Apr 9, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 9, 2021
@kibanamachine @lizozom
[Partial Results] Move inspector adapter integration into search sour…
b4e669e 
…ce (#96241) (#96701)

* Move inspector adapter integration into search source

* docs and ts

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Liza Katz <lizka.k@gmail.com>
phillipb added a commit to phillipb/kibana that referenced this pull request Apr 12, 2021
@phillipb
Merge branch 'master' of github.com:elastic/kibana into add-log-rate-…
b279541 
…to-metrics-tab

* 'master' of github.com:elastic/kibana: (44 commits)
  [Exploratory View]Additional metrics for kpi over time (elastic#96532)
  [Fleet] UI changes on hosted policy detail view (elastic#96337)
  Stacked line charts incorrectly shows one term as 100% (elastic#96203)
  [Fleet] Create enrollment API keys as current user (elastic#96464)
  [Lens] Make table and metric show on top Chart switcher (elastic#96601)
  skip flaky suite (elastic#96691)
  [Lens] Hide "Show more errors" once expanded (elastic#96605)
  [Discover] Unskip histogram hiding test (elastic#95759)
  skip flyout test, add linked issue elastic#96708
  skip copy_to_space_flyout_internal.test.tsx elastic#96708
  fix config validation (elastic#96502)
  Document telemetry fields for stack security features (elastic#96638)
  [Partial Results] Move inspector adapter integration into search source (elastic#96241)
  [RAC] Rule registry plugin (elastic#95903)
  [APM] Run precommit tasks sequentially (elastic#96551)
  [Maps] fix Kibana does not recognize a valid geo_shape index when attempting to create a Tracking Containment alert (elastic#96633)
  [Security Solution] [Cases] Small UI bugfixes (elastic#96511)
  [Actions UI] Changed PagerDuty action form UI to fill payload fields according to the API docs for Resolve and Acknowledge events. (elastic#96363)
  App Search: Result Component Updates (elastic#96184)
  [Alerting] Preconfigured alert history index connector (elastic#94909)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar approved these changes

@majagrubic majagrubic majagrubic approved these changes

Assignees

@lizozom lizozom

Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[data.search.esaggs] Move 'other' bucket & inspector integrations to SearchSouce.
5 participants
@lizozom @elasticmachine @ppisljar @kibanamachine @majagrubic