Move Expression typings from Canvas to OSS Interpreter #36960

lukeelmers · 2019-05-23T04:14:28Z

Summary

This is a first pass at moving the Canvas Expression Function types that @clintandrewhall worked on over to OSS so they can be exported from the interpreter for use by other plugins.

Reviewers: It's probably easiest to follow this by looking at each of the commits in order.

Usage / Testing

import { ExpressionFunction } from 'plugins/interpreter/types';
// ^^ I still need to verify that this webpack alias works in typescript from x-pack

const name = 'myfunc';

interface Context {
  type: 'whatever'; // in most cases you'll be importing Context from elsewhere
}

interface Arguments {
  foo: boolean | null;
}

interface Return {
  type: 'whatever'; // in most cases you'll be importing Return from elsewhere
}

export const myfunc = (): ExpressionFunction<typeof name, Context, Arguments, Return> => ({
  // function definition
});

Notes / Thoughts

Perhaps we should have an interpreter "types" base interface that individual types extend from when authoring their interfaces. Specifically we need to enforce that types have a type string value in their interface (which this generic relies on).
I haven't copied over some of the other generics (ContextFunction and NullContextFunction) because this all seemed to work with one unified generic... but I may be missing edge cases so @clintandrewhall let me know if I'm missing anything!
Next steps if this checks out will be: converting interpreter functions to use the new generic, making sure all types have interfaces defined, removing any types from canvas that this replaces
It may make more sense to re-export from the data plugin now (where this will live long term), instead of interpreter

elasticmachine · 2019-05-23T05:39:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-23T16:01:07Z

Pinging @elastic/kibana-app-arch

elasticmachine · 2019-05-23T16:01:09Z

Pinging @elastic/kibana-canvas

lukeelmers · 2019-05-23T16:02:14Z

I'll mark as ready to review for now, and we can discuss whether (once this is approved) we should include all of the updates in Canvas in this PR, or a subsequent one.

chrisdavies

LGTM w/ possible tweaks. There are a few places where 'any' or 'object' is used, and 'unknown' might be more appropriate. Other than that, this looks great.

chrisdavies · 2019-05-23T18:33:18Z

src/legacy/core_plugins/interpreter/common/types/kibana_datatable.ts

+
+interface Column {
+  id: string;
+  name: string;


Does the column not contain any type information? That'd be pretty handy to have in a number of situations. (e.g. type: 'string')

I don't think so -- kibana_datatable is just the tabified data format, e.g.

{ columns: [{ id: 'foo', name: 'Foo Col' }], rows: [{ foo: 'bar' }, { foo: 'baz' }], }

Chris, yes it'd be handy, it's not present at the moment, but typed table attribs had come up in discussion. For example, in the context of TS, it could warn that one wants to multiply a numeric column with an alpha column, or filter a time attribute represented as milliseconds since epoch with a range that represents time as days since epoch (ie. the physical type matches but the attribute domains have mismatched semantics).

Also, semantic types would be great for something like.... helping determine Kibana Lens options :-) For example, if an editor knew that those numbers represent time in milliseconds, then appropriate time filters etc. can be added (example is made up, only to illustrate that physical, easy to infer types such as numbers don't help a downstream, uncoupled consumer very much). On the demo day prez for shareable dataviz I touched on using attribute types as a goal, and super early steps.

@chrisdavies ^ I should've used the handle

src/legacy/core_plugins/interpreter/common/types/kibana_datatable.ts

chrisdavies · 2019-05-23T18:39:17Z

src/legacy/core_plugins/interpreter/types/common.ts

+/**
+ * Utility type: extracts returned type from a Promise.
+ */
+export type UnwrapPromise<T> = T extends Promise<infer P> ? P : T;


My brain hurts!

Clint was already using this in a generic somewhere, so I just extracted it as a utility. Unfortunately I can't take credit for authorship xD

Use case is when you have a type that may or may not be returned from a Promise -- you can use this and then you don't need to worry about whether the promise is there or not.

Yeah, I understood it. It just took a bit of staring. I was just reminded of my experience learning Haskell a few years back. Hence, the aching brain. :)

elasticmachine · 2019-05-23T21:28:05Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

lukeelmers · 2019-05-23T22:26:40Z

Caused by: org.elasticsearch.common.settings.SettingsException: The configuration setting [xpack.security.authc.realms.oidc.oidc1.rp.client_secret] is required

I think this is an unrelated issue that was fixed on master. Will rebase & push.

elasticmachine · 2019-05-23T23:52:59Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elastic#34252)

* fix code query string key conflict * fix code go to definition url

* remove import from server folder * the other way around too

…c#36563) * Allow to send showLabel parameter with false value * Add unit test

ppisljar · 2019-05-24T06:30:48Z

i tried moving tagcloud fn over to TS, everthing seems to work, i am only missing types for Render type.

…ar intervals (elastic#36081) * auto interval * [TSVB] Rollup Search - 'auto' interval is not working for Rollup Jobs which were created in Calendar intervals * [TSVB] Rollup Search - 'auto' interval is not working for Rollup Jobs which were created in Calendar intervals * fix issue * fix broken tests * fix pr comments * fix pr comments

…#37281) * fix dataprovider query when the filed is a timestamp * review II * review III

…astic#37358)

* [feat] create additional http servers allow for additional http servers to be created, tracked and returned * respond to pr feedback * tweak test * update documentation * destructure port, remove unnecessary imports * [fix] export correct type * [feat] expose createNewServer to plugins * [fix] respond to pr feedback * todo: add schema validation & integration test * use reach * [fix] use validateKey to validate partial * [fix] change config shadowing * check kibana port & prevent shadowing * centralize start/stop for servers, add integration test * remove unnecessary property * never forget your await * remove option to pass config into start * fix pr feedback * fix documentation * fix test failures

* Add renovate.json * Update and rename renovate.json to renovate.json5 * Update renovate.json5 * Update renovate.json5 * extend the base config * remove enabled:false * skip the part limiting managers, we point to specific package.json files instead * try to disable updating all but eslint packages * nest npm specific config * fix syntax error * bad copy job * tweak labels * disable update requests for `@kbn/` packages * stop overriding rangeStategy * extend package.json selectors * add a couple more package.json files

typeof null === 'object', so add an explicit check for null

* Fix formatdate not allowing strings * Addressing Feedback

lukeelmers · 2019-05-29T23:31:10Z

ugh. sorry for the ping everyone 👋

you can safely ignore this

lukeelmers · 2019-05-29T23:39:46Z

To spare y'all from further updates, I'm going to go ahead and close this one in favor of #37438.

elasticmachine · 2019-05-30T00:27:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

lukeelmers changed the title ~~Move typedefs for Expressions to OSS~~ Move Expression typings from Canvas to OSS Interpreter May 23, 2019

lukeelmers added :AppArch Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) review labels May 23, 2019

lukeelmers marked this pull request as ready for review May 23, 2019 16:02

lukeelmers requested review from clintandrewhall, chrisdavies and ppisljar May 23, 2019 16:03

chrisdavies approved these changes May 23, 2019

View reviewed changes

lukeelmers added 4 commits May 23, 2019 16:30

Move Expression types to OSS.

0415303

Add interfaces for KibanaContext and KibanaDatatable types.

2af0f75

Convert esaggs expression function to TypeScript.

14e8043

Clean up KibanaDatatable and KibanaContext types.

248d0ed

lukeelmers force-pushed the feat/interpreter-types branch from 9e3756f to 248d0ed Compare May 23, 2019 22:30

Yulong and others added 6 commits May 24, 2019 10:00

[Code] refactor launcher code, add unit tests (elastic#36863)

1229151

feature(code/frontend): breadcrumb links to code root and project root (

8e53461

elastic#34252)

[Code] fix query string key conflict (elastic#36620)

ff51461

* fix code query string key conflict * fix code go to definition url

[SIEM] Remove imports from server folder (elastic#37037)

8070a1f

* remove import from server folder * the other way around too

moves ui out of index patterns service (elastic#36348)

0259b96

[Tagcloud] Allow to send showLabel parameter with false value (elasti…

80193a1

…c#36563) * Allow to send showLabel parameter with false value * Add unit test

XavierM and others added 8 commits May 29, 2019 13:33

[SIEM] fix data provider query when the field is a timestamp (elastic…

3f78d29

…#37281) * fix dataprovider query when the filed is a timestamp * review II * review III

Add addError function to toastNotifications (elastic#32187)

58ef3a3

bump github-checks-reporter to delay 60s on retries and retry 10x (el…

8f693be

…astic#37358)

[Code] change suggested clone url (elastic#37354)

ddefbe0

Fix: handle null argument values (elastic#37411)

c4c1377

typeof null === 'object', so add an explicit check for null

Fix formatdate not allowing strings (elastic#37301)

5519056

* Fix formatdate not allowing strings * Addressing Feedback

clintandrewhall mentioned this pull request May 29, 2019

[Canvas] Filter is not using the type property correctly; fix in 8.0 #37422

Closed

zfy0701 and others added 4 commits May 29, 2019 15:50

[Code] add util scripts to run tests/checks (elastic#37365)

0965f7c

Update ArgumentString-related types to simply be TypeString.

9169169

Rename types.ts to index.ts so types can be imported at the top level.

a9c0e73

Resolve merge conflicts.

b54e622

lukeelmers requested review from a team as code owners May 29, 2019 23:26

lukeelmers requested a review from a team May 29, 2019 23:26

lukeelmers requested a review from a team as a code owner May 29, 2019 23:26

lukeelmers mentioned this pull request May 29, 2019

Expression typings: Canvas to OSS #37438

Merged

lukeelmers closed this May 29, 2019

timroes mentioned this pull request Jul 5, 2019

[7.2] [Tagcloud] Allow to send showLabel parameter with false value (#36563) #40437

Merged

This was referenced Aug 9, 2019

[7.x] [Monitoring] Used fixed_interval explicitly for date_histogram aggregations (#36960) #43045

Merged

[7.3] [Monitoring] Used fixed_interval explicitly for date_histogram aggregations (#36960) #43046

Merged

lukeelmers deleted the feat/interpreter-types branch February 11, 2021 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Expression typings from Canvas to OSS Interpreter #36960

Move Expression typings from Canvas to OSS Interpreter #36960

lukeelmers commented May 23, 2019 •

edited

Loading

elasticmachine commented May 23, 2019

elasticmachine commented May 23, 2019

elasticmachine commented May 23, 2019

lukeelmers commented May 23, 2019

chrisdavies left a comment

chrisdavies May 23, 2019

lukeelmers May 23, 2019

monfera May 28, 2019

monfera May 28, 2019

chrisdavies May 23, 2019

lukeelmers May 23, 2019

chrisdavies May 24, 2019

elasticmachine commented May 23, 2019

lukeelmers commented May 23, 2019

elasticmachine commented May 23, 2019

ppisljar commented May 24, 2019

lukeelmers commented May 29, 2019

lukeelmers commented May 29, 2019

elasticmachine commented May 30, 2019

Move Expression typings from Canvas to OSS Interpreter #36960

Move Expression typings from Canvas to OSS Interpreter #36960

Conversation

lukeelmers commented May 23, 2019 • edited Loading

Summary

Usage / Testing

Notes / Thoughts

elasticmachine commented May 23, 2019

💚 Build Succeeded

elasticmachine commented May 23, 2019

elasticmachine commented May 23, 2019

lukeelmers commented May 23, 2019

chrisdavies left a comment

Choose a reason for hiding this comment

chrisdavies May 23, 2019

Choose a reason for hiding this comment

lukeelmers May 23, 2019

Choose a reason for hiding this comment

monfera May 28, 2019

Choose a reason for hiding this comment

monfera May 28, 2019

Choose a reason for hiding this comment

chrisdavies May 23, 2019

Choose a reason for hiding this comment

lukeelmers May 23, 2019

Choose a reason for hiding this comment

chrisdavies May 24, 2019

Choose a reason for hiding this comment

elasticmachine commented May 23, 2019

💔 Build Failed

lukeelmers commented May 23, 2019

elasticmachine commented May 23, 2019

💚 Build Succeeded

ppisljar commented May 24, 2019

lukeelmers commented May 29, 2019

lukeelmers commented May 29, 2019

elasticmachine commented May 30, 2019

💚 Build Succeeded

lukeelmers commented May 23, 2019 •

edited

Loading