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.

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

[TSVB] use new Search API for rollup search #82710

Closed
alexwizp opened this issue Nov 5, 2020 · 1 comment · Fixed by #83275
Closed

[TSVB] use new Search API for rollup search #82710

alexwizp opened this issue Nov 5, 2020 · 1 comment · Fixed by #83275
Labels
Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture

Comments

@alexwizp
Copy link
Contributor

alexwizp commented Nov 5, 2020

Historically, the code related to the rollup search in TSVB was added to the x-pack/rollup plugin. And now the rollup plugin has a folder specific only to TSVB (see x-pack/plugins/rollup/server/lib/search_strategies)
IMO, this is a design issue because usually a TSVB plugin should depend on a Rollup plugin, but not vice versa.

On the other hand, in #76274, we started using a new Search API that natively supports search by rollup data.

I think we should:

  1. check which logic we still need from x-pack/plugins/rollup/server/lib/search_strategies and which can be reused from the data plugin
  2. create a new vis_type_timeseries_enhanced plugin and move into it related code
  3. use new Search API for searching by Rollup data
@alexwizp alexwizp added the Feature:TSVB TSVB (Time Series Visual Builder) label Nov 5, 2020
@timroes timroes added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Nov 5, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@timroes timroes added the technical debt Improvement of the software architecture and operational architecture label Nov 5, 2020
alexwizp added a commit to alexwizp/kibana that referenced this issue Nov 13, 2020
alexwizp added a commit that referenced this issue Nov 18, 2020
* [TSVB] use new Search API for rollup search

Closes: #82710

* remove unused code

* rollup_search_strategy.test.js -> rollup_search_strategy.test.ts

* default_search_capabilities.test.js -> default_search_capabilities.test.ts

* remove getRollupService

* fix CI

* fix some types

* update types

* update codeowners

* fix PR comments

Co-authored-by: Kibana Machine <[email protected]>
alexwizp added a commit to alexwizp/kibana that referenced this issue Nov 18, 2020
* [TSVB] use new Search API for rollup search

Closes: elastic#82710

* remove unused code

* rollup_search_strategy.test.js -> rollup_search_strategy.test.ts

* default_search_capabilities.test.js -> default_search_capabilities.test.ts

* remove getRollupService

* fix CI

* fix some types

* update types

* update codeowners

* fix PR comments

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	.github/CODEOWNERS
#	x-pack/plugins/vis_type_timeseries_enhanced/server/search_strategies/rollup_search_capabilities.test.ts
alexwizp added a commit that referenced this issue Nov 18, 2020
* [TSVB] use new Search API for rollup search

Closes: #82710

* remove unused code

* rollup_search_strategy.test.js -> rollup_search_strategy.test.ts

* default_search_capabilities.test.js -> default_search_capabilities.test.ts

* remove getRollupService

* fix CI

* fix some types

* update types

* update codeowners

* fix PR comments

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	.github/CODEOWNERS
#	x-pack/plugins/vis_type_timeseries_enhanced/server/search_strategies/rollup_search_capabilities.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants