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

[data.search.aggs] Remove service getters from agg types #60333

Closed
4 tasks
Tracked by #60126
lukeelmers opened this issue Mar 16, 2020 · 2 comments · Fixed by #61069
Closed
4 tasks
Tracked by #60126

[data.search.aggs] Remove service getters from agg types #60333

lukeelmers opened this issue Mar 16, 2020 · 2 comments · Fixed by #61069
Assignees
@alexwizp
Labels
Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Feature:NP Migration

Comments

@lukeelmers
Copy link
Member

lukeelmers commented Mar 16, 2020 •
edited
Loading

When aggs were migrated to the new platform data.search service, each of the runtime dependencies in the individual agg types were retrieved using service getters/setters (e.g. getUiSettings or getFieldFormats in src/plugins/data/public/services)

In practice there are very few use cases for these getters/setters, and it is good to get rid of them where we can since it makes testing & mocking more complicated.

We already have one example of how these agg types look without the getters/setters if you look at the filtersBucketAgg in buckets/filters: Basically it gets wrapped in a provider function which accepts the needed dependencies, returning the agg type instance.

  • search/aggs/buckets
  • search/aggs/metrics
  • search/aggs/param_types
  • search/aggs/test_helpers/mock_data_services
    • After updating the above items, see if this mocking helper is still necessary.
    • Most likely calls to mockDataServices() in the various unit tests can be removed and replaced with the simpler mocks provided by mocks.ts

Parent issue: #60126
@lukeelmers lukeelmers added Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Team:AppArch Feature:NP Migration labels Mar 16, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@lukeelmers lukeelmers mentioned this issue Mar 16, 2020
Closed
82 tasks
alexwizp added a commit to alexwizp/kibana that referenced this issue Mar 24, 2020
@alexwizp
[data.search.aggs] Remove service getters from agg types
381b1e2 
Part of elastic#60333
@alexwizp alexwizp mentioned this issue Mar 24, 2020
Merged
9 tasks
alexwizp added a commit that referenced this issue Mar 27, 2020
@alexwizp @elasticmachine
[data.search.aggs] Remove service getters from agg types (#61069)
37c4fd4 
* [data.search.aggs] Remove service getters from agg types

Part of #60333

* fix JEST

* fix karma:unit

* fix PR commnets

* fix PR comments

* try to fix ci

* fix CI

* fix karma:unit

Co-authored-by: Elastic Machine <[email protected]>
@alexwizp alexwizp reopened this Mar 27, 2020
alexwizp added a commit to alexwizp/kibana that referenced this issue Mar 27, 2020
@alexwizp @elasticmachine
[data.search.aggs] Remove service getters from agg types (elastic#61069)
34d1e6e 
* [data.search.aggs] Remove service getters from agg types

Part of elastic#60333

* fix JEST

* fix karma:unit

* fix PR commnets

* fix PR comments

* try to fix ci

* fix CI

* fix karma:unit

Co-authored-by: Elastic Machine <[email protected]>
alexwizp added a commit that referenced this issue Mar 27, 2020
@alexwizp @elasticmachine
[data.search.aggs] Remove service getters from agg types (#61069) (#6…
4941c7e 
…1582)

* [data.search.aggs] Remove service getters from agg types

Part of #60333

* fix JEST

* fix karma:unit

* fix PR commnets

* fix PR comments

* try to fix ci

* fix CI

* fix karma:unit

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
@alexwizp alexwizp mentioned this issue Mar 27, 2020
Merged
7 tasks
alexwizp added a commit to alexwizp/kibana that referenced this issue Mar 27, 2020
@alexwizp
[data.search.aggs] Remove service getters from agg types
c193b74 
Part of elastic#60333
elasticmachine added a commit to alexwizp/kibana that referenced this issue Mar 27, 2020
@elasticmachine
Merge branch 'master' into elastic#60333
775eaaf
elasticmachine added a commit to alexwizp/kibana that referenced this issue Mar 27, 2020
@elasticmachine
Merge branch 'master' into elastic#60333
70ccb9c
elasticmachine added a commit to alexwizp/kibana that referenced this issue Mar 28, 2020
@elasticmachine
Merge branch 'master' into elastic#60333
cc74e3a
elasticmachine added a commit to alexwizp/kibana that referenced this issue Mar 30, 2020
@elasticmachine
Merge branch 'master' into elastic#60333
d4b8323
elasticmachine added a commit to alexwizp/kibana that referenced this issue Apr 1, 2020
@elasticmachine
Merge branch 'master' into elastic#60333
845261d
elasticmachine added a commit to alexwizp/kibana that referenced this issue Apr 2, 2020
@elasticmachine
Merge branch 'master' into elastic#60333
b892dfe
alexwizp added a commit to alexwizp/kibana that referenced this issue Apr 3, 2020
@alexwizp
Merge remote-tracking branch 'upstream/master' into elastic#60333
889b150
@VladLasitsa VladLasitsa mentioned this issue Apr 4, 2020
Merged
7 tasks
elasticmachine added a commit to alexwizp/kibana that referenced this issue Apr 6, 2020
@elasticmachine
Merge branch 'master' into elastic#60333
fb3e918
alexwizp added a commit that referenced this issue Apr 7, 2020
@alexwizp @elasticmachine @VladLasitsa
[data.search.aggs] Remove service getters from agg types (#61628)
008b0fd 
* [data.search.aggs] Remove service getters from agg types

Part of #60333

* new portion of changes

* pass dependencies to MetricAgg Type through constructor

* update docs

* refactoring buckets

* remove unused mockDataServices

* Remove service getters from metrics

* Some fixes

* remove temporary code

* moved notifications to the getInternalStartServices

* fixed karma lock

* update docs

* Fixed tests

* fix broken CI

* fix PR comment

* fix typo

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Uladzislau Lasitsa <[email protected]>
alexwizp added a commit to alexwizp/kibana that referenced this issue Apr 7, 2020
@alexwizp @elasticmachine @VladLasitsa
[data.search.aggs] Remove service getters from agg types (elastic#61628)
2b1c217 
* [data.search.aggs] Remove service getters from agg types

Part of elastic#60333

* new portion of changes

* pass dependencies to MetricAgg Type through constructor

* update docs

* refactoring buckets

* remove unused mockDataServices

* Remove service getters from metrics

* Some fixes

* remove temporary code

* moved notifications to the getInternalStartServices

* fixed karma lock

* update docs

* Fixed tests

* fix broken CI

* fix PR comment

* fix typo

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Uladzislau Lasitsa <[email protected]>
alexwizp added a commit that referenced this issue Apr 7, 2020
@alexwizp @elasticmachine @VladLasitsa
[data.search.aggs] Remove service getters from agg types (#61628) (#6…
a83455c 
…2762)

* [data.search.aggs] Remove service getters from agg types

Part of #60333

* new portion of changes

* pass dependencies to MetricAgg Type through constructor

* update docs

* refactoring buckets

* remove unused mockDataServices

* Remove service getters from metrics

* Some fixes

* remove temporary code

* moved notifications to the getInternalStartServices

* fixed karma lock

* update docs

* Fixed tests

* fix broken CI

* fix PR comment

* fix typo

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Uladzislau Lasitsa <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Uladzislau Lasitsa <[email protected]>
@lukeelmers
Copy link
Member Author

Closed by #61628 and #62548

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alexwizp alexwizp

Labels
Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Feature:NP Migration
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[data.search.aggs] Remove service getters from agg types alexwizp/kibana
3 participants
@lukeelmers @elasticmachine @alexwizp