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

[FieldFormatters] Remove aggs format decorator from fieldFormats.deserialize() #106755

Merged
merged 6 commits into from
Jul 28, 2021
Merged

[FieldFormatters] Remove aggs format decorator from fieldFormats.deserialize() #106755

merged 6 commits into from
Jul 28, 2021

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Jul 26, 2021 •
edited
Loading

Summary

Remove dependency field formatters dependency on aggs, this will hopefully allow moving field formatters from data plugin
and help tackle #104367
Closes #71967 (see issue description for more implementation details):

  • Add a flag to FieldFormat that allows you to specify a format as hidden. The idea is that formats with hidden: true would not be surfaced in the UI. These formats are only available in the registry if you get them by ID (e.g. getType('some_hidden_format')), but they would not be available when you are getting collections of formats (e.g. getFieldByType('number')).
  • Register each of the existing custom agg formatters. This includes range, date_range, ip_range, and terms. These formats are be set to hidden: true.
  • Remove fieldFormats.deserialize dependency on aggs.

Checklist

@Dosant Dosant changed the title Dev/remove ff deps on aggs [FieldFormatters] Remove aggs format decorator from fieldFormats.deserialize() Jul 27, 2021
@Dosant Dosant added release_note:skip Skip the PR/issue when compiling release notes Team:AppServices technical debt Improvement of the software architecture and operational architecture v7.15.0 v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Feature:FieldFormatters labels Jul 27, 2021
@Dosant Dosant mentioned this pull request Jul 27, 2021
Closed
@Dosant Dosant marked this pull request as ready for review July 27, 2021 11:30
@Dosant Dosant requested a review from a team as a code owner July 27, 2021 11:30
@elasticmachine
Copy link
Contributor

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

@Dosant Dosant requested a review from mattkime July 27, 2021 11:30
mattkime
mattkime approved these changes Jul 27, 2021
View reviewed changes
Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

changes lgtm, but I'm pretty new to aggs. do you want to get the opinion of someone with more aggs experience?

@Dosant
Copy link
Contributor Author

Dosant commented Jul 28, 2021 •
edited
Loading

@mattkime I think this one is pretty safe because the plan was laid out by @lukeelmers and I followed it 90% 🙏 #71967

@ppisljar, but could you please take another look for a piece of mind?

@Dosant Dosant requested a review from ppisljar July 28, 2021 09:40
@Dosant
Copy link
Contributor Author

Dosant commented Jul 28, 2021

This change will also help to fix #106839

ppisljar
ppisljar approved these changes Jul 28, 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. didn't test it out

@Dosant
Merge branch 'master' of github.com:elastic/kibana into dev/remove-ff…
4369583 
…-deps-on-aggs

# Conflicts:
#	src/plugins/data/public/public.api.md
@Dosant
update docs
586ef76
@Dosant
Merge branch 'master' of github.com:elastic/kibana into dev/remove-ff…
7d8874c 
…-deps-on-aggs

# Conflicts:
#	src/plugins/data/common/field_formats/field_formats_registry.ts
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
data 597 596 -1

Page load bundle

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

id before after diff
data 840.6KB 841.0KB +421.0B
Unknown metric groups

API count

id before after diff
data 3741 3743 +2

History

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

@Dosant Dosant merged commit 6559bf4 into elastic:master Jul 28, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 28, 2021
@Dosant @kibanamachine
[FieldFormatters] Remove aggs format decorator from `fieldFormats.des…
d140bd5 
…erialize()` (elastic#106755)
@kibanamachine kibanamachine mentioned this pull request Jul 28, 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 Jul 28, 2021
@kibanamachine @Dosant
[FieldFormatters] Remove aggs format decorator from `fieldFormats.des…
c52675e 
…erialize()` (#106755) (#107031)

Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021
@Dosant @vadimkibana
[FieldFormatters] Remove aggs format decorator from `fieldFormats.des…
d5bfcf6 
…erialize()` (elastic#106755)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattkime mattkime mattkime approved these changes

@ppisljar ppisljar ppisljar approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Feature:FieldFormatters release_note:skip Skip the PR/issue when compiling release notes technical debt Improvement of the software architecture and operational architecture v7.15.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[data.fieldFormats] Remove aggs format decorator from fieldFormats.deserialize()
5 participants
@Dosant @elasticmachine @kibanamachine @mattkime @ppisljar