Index pattern public api => common #68289

mattkime · 2020-06-04T16:57:08Z

Summary

This pr moves the index pattern api into a common directory and factors out client side code. The next pr will expose index pattern api via server.

Part of moving the index pattern api to a common (client and server) api - #68003

Replace core.notifications.toast with onNotification and onError callback functions
Replace FieldFormatsStart usage with FieldFormatsStartCommon
Move kibana_utils/public/{errors, field_mapping} => kibana_utils/common/{errors, field_mapping}
Move Index Pattern API code to common directory

This reverts commit 5de1b21.

This reverts commit ebc56d8.

elasticmachine · 2020-06-06T00:50:39Z

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

ppisljar

left some initial comments

ppisljar · 2020-06-08T09:50:27Z

src/plugins/data/common/index_patterns/fields/field.ts

-        title,
-        text,
-      });
+      onNotification({ title, text, color: 'danger', iconType: 'alert' });


what if we throw here and let the caller handle the exception. instead of passing onNotification in they can wrap code in try/cache

This is good idea but I'd prefer to address it in another ticket as this PR aims to be a minimal set of changes required to provide a common api. I think I'd need to examine all places where index patterns are used to ensure errors are being caught. I've added a ticket to #67920

ppisljar · 2020-06-08T09:51:41Z

src/plugins/data/common/index_patterns/fields/field_list.ts

  specs?: FieldSpec[],
  shortDotsEnable?: boolean
 ) => IIndexPatternFieldList;

 export const getIndexPatternFieldListCreator = ({
  fieldFormats,
-  toastNotifications,
+  onNotification,


on this level we can then remove onNotification and leave catching the exception to the caller.

ppisljar · 2020-06-08T09:54:06Z

src/plugins/data/common/index_patterns/index_patterns/index_pattern.ts

          return [];
        }

-        toasts.addError(err, {
+        this.onError(err, {


lets throw here as well and leave it up to the caller to decide what kind of notification they want to show ?

ppisljar · 2020-06-08T09:57:11Z

src/plugins/data/common/index_patterns/index_patterns/ensure_default_index_pattern.ts

+      defaultId = patterns[0];
+      core.uiSettings.set('defaultIndex', defaultId);
+    } else {
+      return onRedirectNoIndexPattern();


lets have this method always return and the caller decide what to do (no need to pass in callbacks)

ppisljar · 2020-06-08T10:03:40Z

src/plugins/data/common/index_patterns/index_patterns/ensure_default_index_pattern.ts

+export type EnsureDefaultIndexPattern = () => Promise<unknown> | undefined;
+
+export const createEnsureDefaultIndexPattern = (
+  core: CoreStart,


lets not pass in whole core but just uiSettingsClient

is there a reason this is wrapped in a function ? why not just export ensureDefaultIndexPattern ?

I think I'd prefer to leave further refactoring of this code to future PRs. I'm drawing a rather arbitrary line since I need to do things backwards - provide the service and then clean up the code.

mattkime · 2020-06-08T10:52:26Z

@elasticmachine merge upstream

This reverts commit 30bf1e0.

…na into index_pattern_server_api_2

lukeelmers

Overall makes sense, my main questions were around keeping some of the old directories in public which are only re-exporting common items & could probably be removed.

Also I agree with @ppisljar's idea on getting rid of the callbacks & instead just throwing and relying on the caller to handle things appropriately. But no strong feelings as to whether that's addressed here or as a follow-up.

lukeelmers · 2020-06-08T23:19:14Z

src/plugins/data/public/index_patterns/fields/index.ts

@@ -17,5 +17,4 @@
 * under the License.
 */

-export * from './field_list';
-export * from './field';
+export * from '../../../common/index_patterns/fields';


Rather than re-exporting here, should we just remove the public/index_patterns/fields directory entirely and import items from common/index_patterns/fields wherever they are needed?

These haven't been removed yet because I ran into errors when I did them all at once. There's an undesirable import somewhere. Just a matter of chasing it down now or later.

lukeelmers · 2020-06-08T23:20:36Z

src/plugins/data/public/index_patterns/index_patterns/index.ts

-export * from './index_patterns';
-export * from './index_patterns_api_client';
-export * from './types';
+export * from '../../../common/index_patterns/index_patterns';


Same question, should we re-export from here?

Addressed other instances but this one is giving me trouble.

src/plugins/data/public/index_patterns/lib/index.ts

src/plugins/kibana_utils/common/index.ts

mattkime · 2020-06-09T03:17:35Z

But no strong feelings as to whether that's addressed here or as a follow-up.

No strong opinions here either. Drew the line here because the PR was still relatively easy to understand. Perhaps we should determine where to draw the line by when works starts on the index pattern expression function. I was under the impression that the answer was 'as soon as possible'

mattkime · 2020-06-09T18:12:12Z

@elasticmachine merge upstream

…na into index_pattern_server_api_2

This reverts commit 885c8ce.

This reverts commit 0b7f84f.

kibanamachine · 2020-06-10T03:30:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 985e90a

History

💚 Build #52894 succeeded e3a16b3
💚 Build #52856 succeeded ace29ae
💚 Build #52805 succeeded 5beadc0
💔 Build #52762 failed f96d3c3
💔 Build #52736 failed 6c30c28

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

ppisljar

code LGTM

lukeelmers

Code LGTM -- I'm cool with addressing the items discussed in a followup PR.

Tested Chrome (macOS) and everything seemed to work as expected 👍

I was able to reproduce the "missing indices" error message using the onNotification callback, though I did not test every notification or error scenario.

Refactor index pattern api so it can be used by public and server services # Conflicts: # src/plugins/data/common/index_patterns/index_patterns/index_pattern.ts # src/plugins/data/public/index_patterns/index_patterns/index_pattern.ts # src/plugins/data/public/index_patterns/index_patterns/index_pattern.tsx

* master: (38 commits) Support migrating from reserved feature privileges (elastic#68504) add `preference` field to SavedObjectsFindOptions (elastic#68620) [ILM] Add "wait for snapshot" policy field to Delete phase (elastic#68505) Cleanup old license overwrites (elastic#68744) Bump TypeScript to v3.9 (elastic#67666) [APM] Service maps - adds new storybook stories to test out various data sets (elastic#68727) Fix vega specification parsing (elastic#67963) docs: add more api information (elastic#68717) [APM] Don't show annotations on charts with no data (elastic#68829) [Metrics UI] Fix Inventory View sorting by handling null values (elastic#67889) skip flaky suite (elastic#68836) [SIEM][Detections Engine] - Fix reference rule url overflow (elastic#68640) Index pattern public api => common (elastic#68289) [APM] Lazy-load alert triggers (elastic#68806) [DOCS] Fix table formatting in ingest manager settings (elastic#68824) [Endpoint] Functional Tests cleanup (elastic#68756) revert previous commit which was unintentional Use Github token instead for project assignments [SIEM][Exceptions] - ExceptionsViewer cleanup (elastic#68739) move @kbn/storybook to devDeps (elastic#68791) ...

* Index pattern public api => common (#68289)

kibanamachine · 2020-06-12T22:28:45Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 68289 or prevent reminders by adding the backport:skip label.

mattkime added 10 commits June 4, 2020 11:54

partial progress

a2da8d3

front end code should be working again

3a40208

restore unknown field toast

cb9e775

fix jest test mock

5de1b21

Revert "fix jest test mock"

2683b7a

This reverts commit 5de1b21.

fix jest test mock

2913835

add onError callback, remove comments

5cfaee5

Merge branch 'master' into index_pattern_server_api_2

f963496

better define fieldFormats common start service

ec2f0c6

dependency cleanup

ebc56d8

mattkime changed the title ~~index pattern server api~~ Index pattern public api => common Jun 5, 2020

Revert "dependency cleanup"

c994a1f

This reverts commit ebc56d8.

mattkime marked this pull request as ready for review June 6, 2020 00:06

mattkime requested a review from a team as a code owner June 6, 2020 00:06

mattkime added the Team:AppArch label Jun 6, 2020

mattkime added Feature:Data Views Data Views code and UI - index patterns before 8.0 v8.0.0 v7.9.0 release_note:skip Skip the PR/issue when compiling release notes labels Jun 6, 2020

mattkime mentioned this pull request Jun 6, 2020

Index Pattern API to server #68003

Closed

8 tasks

update api docs

f097b31

elastic deleted a comment from kibanamachine Jun 6, 2020

mattkime added 3 commits June 5, 2020 20:04

remove commented lines

6998499

typefix

01e6e2f

refactor core usage

f75873f

ppisljar requested changes Jun 8, 2020

View reviewed changes

mattkime added 3 commits June 8, 2020 17:05

Revert "more specific dependencies"

bce0d2e

This reverts commit 30bf1e0.

more specific core usage

90a8695

Merge branch 'index_pattern_server_api_2' of github.com:mattkime/kiba…

240371a

…na into index_pattern_server_api_2

lukeelmers reviewed Jun 8, 2020

View reviewed changes

more specific core usage

d8b1999

mattkime added 2 commits June 9, 2020 10:05

move field_mapping code from kibana_utils to data

6c30c28

fix jest test mock

f96d3c3

elasticmachine and others added 8 commits June 9, 2020 11:12

Merge branch 'master' into index_pattern_server_api_2

5beadc0

remove fields index file, was only re-exporting from common

7aeb8f2

Merge branch 'index_pattern_server_api_2' of github.com:mattkime/kiba…

ace29ae

…na into index_pattern_server_api_2

remove index pattern lib dir which just re-exports from common

e3a16b3

remove re-export

0b7f84f

remove re-export

885c8ce

Revert "remove re-export"

03e849d

This reverts commit 885c8ce.

Revert "remove re-export"

985e90a

This reverts commit 0b7f84f.

ppisljar approved these changes Jun 10, 2020

View reviewed changes

lukeelmers approved these changes Jun 10, 2020

View reviewed changes

mattkime merged commit 40d2cfc into elastic:master Jun 10, 2020

mattkime mentioned this pull request Jun 11, 2020

[7.x] Index pattern public api => common (#68289) #68843

Closed

mattkime added a commit that referenced this pull request Jun 11, 2020

[7.x] Index pattern public api => common (#68289) (#68863)

5d55413

* Index pattern public api => common (#68289)

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 12, 2020

mattkime added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Jun 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index pattern public api => common #68289

Index pattern public api => common #68289

mattkime commented Jun 4, 2020 •

edited

Loading

elasticmachine commented Jun 6, 2020

ppisljar left a comment

ppisljar Jun 8, 2020

mattkime Jun 8, 2020

ppisljar Jun 8, 2020

ppisljar Jun 8, 2020

ppisljar Jun 8, 2020

ppisljar Jun 8, 2020

ppisljar Jun 8, 2020

mattkime Jun 8, 2020

mattkime commented Jun 8, 2020

lukeelmers left a comment

lukeelmers Jun 8, 2020

mattkime Jun 9, 2020

lukeelmers Jun 8, 2020

mattkime Jun 10, 2020

mattkime commented Jun 9, 2020

mattkime commented Jun 9, 2020

kibanamachine commented Jun 10, 2020

ppisljar left a comment

lukeelmers left a comment •

edited

Loading

kibanamachine commented Jun 12, 2020

Index pattern public api => common #68289

Index pattern public api => common #68289

Conversation

mattkime commented Jun 4, 2020 • edited Loading

Summary

elasticmachine commented Jun 6, 2020

ppisljar left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattkime commented Jun 8, 2020

lukeelmers left a 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

mattkime commented Jun 9, 2020

mattkime commented Jun 9, 2020

kibanamachine commented Jun 10, 2020

💚 Build Succeeded

History

ppisljar left a comment

Choose a reason for hiding this comment

lukeelmers left a comment • edited Loading

Choose a reason for hiding this comment

kibanamachine commented Jun 12, 2020

mattkime commented Jun 4, 2020 •

edited

Loading

lukeelmers left a comment •

edited

Loading