Move Timefilter service ⇒ NP #49491

lizozom · 2019-10-28T16:37:13Z

Summary

Move timefilter ⇒ NP

Dev Docs

Moved the timefilter service to New Platform.

Usage in old platform:

import { TimeRange } from 'src/plugins/data/public';
import { npSetup, npStart } from 'ui/new_platform';

const { timefilter } = npStart.data.timefilter;
const timeRange: TimeRange = timefilter.getTime();
const refreshInterval: RefreshInterval = timefilter.getRefreshInterval()

Usage in new platform:

class MyPlugin {
   public setup(core: CoreSetup, { data }: MyPluginSetupDependencies) {
      const timeRange: TimeRange = data.timefilter.timefilter.getTime();
   }
   public start(core: CoreStart, { data }: MyPluginStartDependencies) {
      const newTimeRange = { from: getYesterday(), to: getNow() }
      data.timefilter.timefilter.setTime(newTimeRange);
   }
}

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-10-28T17:58:02Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: db7341a5cc185b88e595e422d5b0ef6a4260a2d5

elasticmachine · 2019-10-29T11:48:35Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: c1644073588332f132d2134398639780cbb2ab02

elasticmachine · 2019-10-29T13:59:58Z

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

elasticmachine · 2019-10-29T14:15:20Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: cb4042bd6db02d0a65044497982d45c55ad96742

elasticmachine · 2019-10-29T15:43:38Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: bfb5b76e62f43f61604aa71fd0e5439b4d9e266d

elasticmachine · 2019-10-30T13:56:30Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 6aea0e3

elasticmachine · 2019-10-30T15:49:25Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1ab3034

elasticmachine · 2019-10-30T16:34:34Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 0ece01a

peteharverson

ML edit to the chart_utils test LGTM

streamich

These two seem to be blockers:

Importing Index Patterns in the NP data plugin from the LP.
Removing type safety in mocks.

Other comments are for discussion.

streamich · 2019-10-31T09:59:32Z

src/legacy/core_plugins/data/public/plugin.ts

@@ -44,7 +43,6 @@ export interface DataPluginStartDependencies {
 */
 export interface DataSetup {
  query: QuerySetup;
-  timefilter: TimefilterSetup;


Instead of removing timefilter from the shim, shall we re-export the timefilter from the NP here.

/cc @ppisljar

I prefer the opposite approach and I've been using it in previous PR.
I've been working with other teams and promotes faster adaptation.
And it allows us to maintain less code.

streamich · 2019-10-31T10:01:02Z

src/legacy/core_plugins/timelion/public/legacy.ts

 import { TimelionPluginSetupDependencies } from './plugin';
 import { LegacyDependenciesPlugin } from './shim';

 const setupPlugins: Readonly<TimelionPluginSetupDependencies> = {
  visualizations,
-  data,
+  data: npSetup.plugins.data,


If timefilter was re-exported in the shim, this change would not be necessary. Also, now if Timelion needs some other service besides timefilter from the data plugin they would need to import the shim and the NP plugin, merge those locally, also import types of both of those and merge those locally.

src/legacy/core_plugins/timelion/public/plugin.ts

streamich · 2019-10-31T10:06:24Z

src/legacy/ui/public/timefilter/index.ts

-export const timeHistory = data.timefilter.history;
-export const timefilter = data.timefilter.timefilter;
+export const timeHistory = npStart.plugins.data.query.timefilter.history;
+export const timefilter = npStart.plugins.data.query.timefilter.timefilter;


Is the double timefilter key — .timefilter.timefilter — temporary, or we are planning to keep that?

ATM it's just backwards compatible.
Though I'm not a big fan as well.
I'm open to making a follow up PR updating the structure.

src/plugins/data/public/mocks.ts

src/plugins/data/public/query/mocks.ts

streamich · 2019-10-31T10:22:36Z

src/plugins/data/public/query/timefilter/get_time.ts

-import { IndexPattern, Field } from '../index_patterns';
+
+// TODO: remove this
+import { IndexPattern, Field } from '../../../../../legacy/core_plugins/data/public/index_patterns';


This seems like a no-no—importing from the Legacy Platform in the New Platform. My understanding is that we should not be doing this even with TODO comment, but maybe I'm wrong (/cc @ppisljar @joshdover ).

Talked about this with @ppisljar.
We decided to go with it, because otherwise we're blocked until IndexPatterns are done. It's a temporary solution that works since it's only a type, and once IndexPatterns are migrated, we'll switch it over.

its only types that we are importing, so this should be ok

x-pack/legacy/plugins/siem/public/components/search_bar/index.tsx

…plugin/move-timefilter-to-NP

elasticmachine · 2019-10-31T12:23:21Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: acf5ddd

…plugin/move-timefilter-to-NP

elasticmachine · 2019-11-06T23:16:14Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: b9c2505

…plugin/move-timefilter-to-NP

lizozom · 2019-11-10T15:38:01Z

retest

elasticmachine · 2019-11-10T17:07:52Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a503787

streamich

Talked with @ppisljar, he is OK for this PR to import legacy Index Patterns from a New Platform plugin, thus removing request for changes.

@ppisljar

Talked with @ppisljar, he is OK for this PR to import legacy Index Patterns from a New Platform plugin, thus removing request for changes.

lizozom · 2019-11-11T11:54:58Z

@streamich thanks, if so is this approved?

streamich · 2019-11-11T12:19:52Z

@lizozom I did not approve. My understanding of the New Platform migration is that we should never import code from the Legacy Platform in a New Platform plugin.

But @ppisljar seems to be OK with the import in this PR, so I unblocked it in case he wants to approve.

lizozom · 2019-11-11T12:26:53Z

Don't you feel it's kind of redundant? Anyway @ppisljar could you look then?

ppisljar

code LGTM

* Move timefilter to NP * Fixed editor import * added karma mocks * More karma mocks * Change way timefilter is mocked * Fix timefilter passing in timelion * attempted revert of mock types

lizozom self-assigned this Oct 28, 2019

lizozom force-pushed the newplatform/data-plugin/move-timefilter-to-NP branch from db7341a to a51d68d Compare October 29, 2019 10:20

lizozom force-pushed the newplatform/data-plugin/move-timefilter-to-NP branch from c164407 to cb4042b Compare October 29, 2019 13:02

lizozom marked this pull request as ready for review October 29, 2019 13:57

lizozom requested a review from a team as a code owner October 29, 2019 13:57

lizozom added Feature:NP Migration release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:AppArch v7.6.0 v8.0.0 labels Oct 29, 2019

lizozom mentioned this pull request Oct 29, 2019

Move Filter \ Timefilter \ Query Services ⇒ NP #44377

Closed

26 tasks

Liza K added 3 commits October 30, 2019 14:39

Move timefilter to NP

a1407c8

Fixed editor import

4e28d4e

added karma mocks

6aea0e3

lizozom force-pushed the newplatform/data-plugin/move-timefilter-to-NP branch from bfb5b76 to 6aea0e3 Compare October 30, 2019 12:39

More karma mocks

0ece01a

Change way timefilter is mocked

1ab3034

lizozom requested a review from a team as a code owner October 30, 2019 14:28

lizozom changed the title ~~Move timefilter ⇒ NP~~ Move timefilter and PersistedLog ⇒ NP Oct 30, 2019

peteharverson approved these changes Oct 30, 2019

View reviewed changes

streamich previously requested changes Oct 31, 2019

View reviewed changes

Liza K added 2 commits October 31, 2019 12:57

Merge remote-tracking branch 'upstream/master' into newplatform/data-…

584bf68

…plugin/move-timefilter-to-NP

Fix timefilter passing in timelion

acf5ddd

Merge remote-tracking branch 'upstream/master' into newplatform/data-…

b9c2505

…plugin/move-timefilter-to-NP

Merge remote-tracking branch 'upstream/master' into newplatform/data-…

af46a65

…plugin/move-timefilter-to-NP

attempted revert of mock types

a503787

streamich requested review from streamich and removed request for streamich November 11, 2019 11:41

streamich reviewed Nov 11, 2019

View reviewed changes

ppisljar approved these changes Nov 11, 2019

View reviewed changes

lizozom merged commit a72cb85 into elastic:master Nov 11, 2019

lizozom mentioned this pull request Nov 11, 2019

[7.x] Move timefilter and PersistedLog ⇒ NP (#49491) #50152

Merged

lizozom deleted the newplatform/data-plugin/move-timefilter-to-NP branch November 14, 2019 13:03

lizozom changed the title ~~Move timefilter and PersistedLog ⇒ NP~~ Move Timefilter service ⇒ NP Jan 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Timefilter service ⇒ NP #49491

Move Timefilter service ⇒ NP #49491

lizozom commented Oct 28, 2019 •

edited

Loading

elasticmachine commented Oct 28, 2019

elasticmachine commented Oct 29, 2019

elasticmachine commented Oct 29, 2019

elasticmachine commented Oct 29, 2019

elasticmachine commented Oct 29, 2019

elasticmachine commented Oct 30, 2019

elasticmachine commented Oct 30, 2019

elasticmachine commented Oct 30, 2019

peteharverson left a comment

streamich left a comment •

edited by lizozom

Loading

streamich Oct 31, 2019

streamich Oct 31, 2019

lizozom Oct 31, 2019

streamich Oct 31, 2019

streamich Oct 31, 2019

lizozom Oct 31, 2019

streamich Oct 31, 2019

lizozom Oct 31, 2019 •

edited

Loading

ppisljar Nov 11, 2019

elasticmachine commented Oct 31, 2019

elasticmachine commented Nov 6, 2019

lizozom commented Nov 10, 2019

elasticmachine commented Nov 10, 2019

streamich left a comment

lizozom commented Nov 11, 2019

streamich commented Nov 11, 2019

lizozom commented Nov 11, 2019

ppisljar left a comment

Move Timefilter service ⇒ NP #49491

Move Timefilter service ⇒ NP #49491

Conversation

lizozom commented Oct 28, 2019 • edited Loading

Summary

Dev Docs

Checklist

For maintainers

elasticmachine commented Oct 28, 2019

💔 Build Failed

elasticmachine commented Oct 29, 2019

💔 Build Failed

elasticmachine commented Oct 29, 2019

elasticmachine commented Oct 29, 2019

💔 Build Failed

elasticmachine commented Oct 29, 2019

💔 Build Failed

elasticmachine commented Oct 30, 2019

💔 Build Failed

elasticmachine commented Oct 30, 2019

💚 Build Succeeded

elasticmachine commented Oct 30, 2019

💔 Build Failed

peteharverson left a comment

Choose a reason for hiding this comment

streamich left a comment • edited by lizozom Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lizozom Oct 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 31, 2019

💚 Build Succeeded

elasticmachine commented Nov 6, 2019

💚 Build Succeeded

lizozom commented Nov 10, 2019

elasticmachine commented Nov 10, 2019

💚 Build Succeeded

streamich left a comment

Choose a reason for hiding this comment

lizozom commented Nov 11, 2019

streamich commented Nov 11, 2019

lizozom commented Nov 11, 2019

ppisljar left a comment

Choose a reason for hiding this comment

lizozom commented Oct 28, 2019 •

edited

Loading

streamich left a comment •

edited by lizozom

Loading

lizozom Oct 31, 2019 •

edited

Loading