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

Clean up date histogram agg type. #58805

Merged
merged 13 commits into from
Mar 5, 2020
Merged

Clean up date histogram agg type. #58805

merged 13 commits into from
Mar 5, 2020

Conversation

lukeelmers
Copy link
Member

@lukeelmers lukeelmers commented Feb 28, 2020 •
edited
Loading

Part of #42639

Summary

This PR cleans up the remaining items from the date histogram agg type which are preventing us from doing a cutover to the new platform data plugin.

It also converts ui/time_buckets to TS and makes it internal to the data plugin. The only other place ui/time_buckets was in use (where it hadn't been simply copied into the plugin), is in Lens, which was using it for some utilities supporting the autoDate expression function. I've extracted those utilities and, with a few modifications, provided them as part of the data plugin contract for lens to consume instead. This removes the need for lens to rely on the legacy TimeBuckets, and also provides some useful functionality which we will eventually want to incorporate elsewhere (e.g. search/esaggs).

There are a handful of follow-up tasks that will need to happen (listed below), but none of these should block our ability to cutover to the new platform first, which means downstream teams can continue with migration. We can finish the rest of the cleanup from the new platform.

Tasks:

  • convert ui/time_buckets to TS
    • timebuckets
    • calc_es_interval
  • move timebuckets into data shim plugin
    • move ui/ files
    • make static
  • move auto date helpers out of lens (so timebuckets is not required)
    • toAbsoluteDates
    • calculateAutoTimeExpression
  • setBounds
    • make static
    • build_pipeline: remove usage of setBounds
      • ran into a few issues on this, so it was cleaned up but will need to removed entirely later
    • build_pipeline: look into making formatter util
      • ran into some issues on this too; will investigate as a follow up
  • Update dev docs (done in Dev Docs entry: ui/public cleanup 7.7 #55926)

Items to address after cutover:

  • Additional clean up to TimeBuckets types.
    • remove duplicated interface in the date_histogram agg type
    • refactor to make typing easier / better
    • sort out a better way to handle typing for moment.Duration (which you an perform math on)
  • get rid of __cached__ if possible
  • Write time_buckets.ts unit tests

@lukeelmers lukeelmers self-assigned this Feb 28, 2020
@lukeelmers lukeelmers changed the title Clean up agg_types static exports. Clean up date histogram agg type. Mar 1, 2020
@lukeelmers lukeelmers mentioned this pull request Mar 1, 2020
Closed
14 tasks
@lukeelmers lukeelmers added Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Feature:NP Migration release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:AppArch v7.7.0 v8.0.0 WIP Work in progress labels Mar 3, 2020
@elasticmachine
Copy link
Contributor

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

@Dosant Dosant mentioned this pull request Mar 3, 2020
Merged
7 tasks
@lukeelmers lukeelmers force-pushed the aggs/static branch 2 times, most recently from 3ce32a3 to 3994828 Compare March 4, 2020 03:52
@lukeelmers lukeelmers added review and removed WIP Work in progress labels Mar 4, 2020
@lukeelmers lukeelmers marked this pull request as ready for review March 4, 2020 04:11
@lukeelmers lukeelmers requested a review from a team March 4, 2020 04:11
@lukeelmers lukeelmers requested a review from a team as a code owner March 4, 2020 04:11
@lukeelmers lukeelmers removed the request for review from a team March 4, 2020 20:28
@lukeelmers lukeelmers mentioned this pull request Mar 4, 2020
Merged
7 tasks
wylieconlon
wylieconlon approved these changes Mar 4, 2020
View reviewed changes
Copy link
Contributor

@wylieconlon wylieconlon 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!

x-pack/legacy/plugins/lens/public/editor_frame_service/merge_tables.ts Outdated Show resolved Hide resolved
x-pack/legacy/plugins/lens/public/indexpattern_datasource/auto_date.ts Outdated Show resolved Hide resolved
ppisljar
ppisljar approved these changes Mar 4, 2020
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

src/legacy/core_plugins/visualizations/public/np_ready/public/legacy/build_pipeline.ts Outdated Show resolved Hide resolved
@lukeelmers lukeelmers mentioned this pull request Mar 4, 2020
Closed
@lukeelmers lukeelmers added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. labels Mar 4, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@lukeelmers lukeelmers merged commit ba5784a into elastic:master Mar 5, 2020
@lukeelmers lukeelmers deleted the aggs/static branch March 5, 2020 00:04
@lukeelmers lukeelmers mentioned this pull request Mar 5, 2020
Merged
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 5, 2020
@lukeelmers
Clean up date histogram agg type. (elastic#58805)
bea4b73
lukeelmers added a commit that referenced this pull request Mar 5, 2020
@lukeelmers
Clean up date histogram agg type. (#58805) (#59390)
3905bea
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 5, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into console/featu…
ce71db9 
…re/files-and-filetree

* 'master' of github.com:elastic/kibana: (254 commits)
  Convert discover_page to ts, remove redundunt methods (elastic#59312)
  [Fix for Vis Editor] Revert setting time field to empty string when it's undefined (elastic#58873)
  Delete legacy search endpoint (elastic#59341)
  [Uptime] Improve duration chart (elastic#58404)
  [Snapshot & Restore] NP migration (elastic#59109)
  [ML] Add support for date_nanos time field in anomaly job wizard (elastic#59017)
  Revert "Makes alerting and actions optional properties for interface RequestH… (elastic#59264)"
  Change remote_clusters ID to remoteClusters (elastic#59246)
  Makes alerting and actions optional properties for interface RequestH… (elastic#59264)
  Clean up date histogram agg type. (elastic#58805)
  [ML] Management: fix license unsubscribe (elastic#59365)
  Remove documentation for server.cors settings (elastic#59096)
  Edit alert flyout (elastic#58964)
  [SIEM] Fix rule delete/duplicate actions (elastic#59306)
  move mouse to close obstructing tooltip (elastic#59214)
  Reset page after deleting (elastic#59310)
  Make sure phrases input filter triggers autosuggestons (elastic#59299)
  Add loading count source for http requests (elastic#59245)
  Revert "[ML] Transforms: Deprecate custom KibanaContext. (elastic#59133)"
  Expose metrics service to public API (elastic#59294)
  ...

# Conflicts:
#	src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx
@lukeelmers lukeelmers mentioned this pull request Mar 5, 2020
Closed
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

@wylieconlon wylieconlon wylieconlon approved these changes

Assignees

@lukeelmers lukeelmers

Labels
Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes review v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lukeelmers @elasticmachine @kibanamachine @wylieconlon @ppisljar