[Expressions] Refactor expression functions to use observables underneath #100409

dokmic · 2021-05-20T15:57:56Z

Summary

Refactor expression functions to use observables underneath.
Resolves #93016.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2021-05-27T07:54:24Z

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

src/plugins/expressions/common/expression_functions/specs/map_column.ts

ppisljar · 2021-05-31T12:40:52Z

src/plugins/expressions/common/expression_functions/specs/map_column.ts

+                  columns: [...input.columns],
+                  rows: [row],
+                }) ?? of(null)
+              ).pipe(map((value) => ({ ...row, [id]: value })))


i think we don't need the pipe here

We do need that because the expression returns a value that we need to put into the row. The expression is an observable, so we have to use the pipe here to process the value upon subscription.

ppisljar · 2021-05-31T12:41:54Z

src/plugins/expressions/common/expression_functions/specs/map_column.ts

-    });
+    return defer(() => {
+      const rows$ = input.rows.length
+        ? zip(


zip will wait for all rows to emit the first value .... should we consider emitting rows as they come ?

I guess that behavior will be backward incompatible and may not work as expected without adapting the code in other plugins.

ppisljar · 2021-05-31T12:42:28Z

src/plugins/expressions/common/expression_functions/specs/map_column.ts

-        const metaSourceFrom = columns.find(({ id }) => id === args.copyMetaFrom);
-        newColumn.meta = { ...newColumn.meta, ...(metaSourceFrom?.meta || {}) };
-      }
+      return rows$.pipe<Datatable>(


why do we need to pipe here?

That's here because we need to process rows altogether and put them into a datatable.

ppisljar · 2021-05-31T12:45:36Z

x-pack/plugins/canvas/canvas_plugin_src/functions/common/case.ts

-      return { type: 'case', matches, result };
+    fn(input, { if: condition, then, when }) {
+      return defer(() => {
+        const matches = condition ?? when?.().pipe(map((value) => value === input)) ?? true;


when returns an observable. That's the cleanest way to convert the value.

x-pack/plugins/canvas/canvas_plugin_src/functions/common/case.ts

ppisljar · 2021-05-31T12:48:01Z

x-pack/plugins/canvas/canvas_plugin_src/functions/common/if.ts

-        return args.else?.().pipe(take(1)).toPromise() ?? input;
-      }
+    fn(input, args) {
+      return defer(() => (args.condition ? args.then?.() : args.else?.()) ?? of(input));


condition is required

Corrected the argument type, but the code remains the same because then and else are optional.

ppisljar · 2021-05-31T12:48:43Z

x-pack/plugins/canvas/canvas_plugin_src/functions/common/ply.ts

-        byColumns = args.by.map((by) => {
-          const column = input.columns.find((col) => col.name === by);
+      const byColumns =
+        args.by?.map((by) => {


by will always exist (will be empty array if not set)

I've checked that, and optional multi-valued arguments are actually undefined.

flash1293

Tested with Lens formula PR (which uses mapColumn) and everything works fine.

poffdeluxe

Tested this out across a few different workpads that used the functions in question. Everything seems to be working fine!

ppisljar

LGTM

kibanamachine · 2021-06-10T22:18:02Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`canvas`	1.3MB	1.3MB	+2.0B

Page load bundle

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

id	before	after	diff
`canvas`	530.9KB	531.2KB	+285.0B
`expressions`	202.5KB	202.7KB	+281.0B
total			+566.0B

History

💔 Build #130613 failed 394fe55
💔 Build #130544 failed dc5e04c
💚 Build #127494 succeeded 3b09f0d
💔 Build #126745 failed 47d6b35

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

…eath (#100409) (#101988)

* master: (68 commits) skip flaky suite (elastic#94043) skip flaky suite (elastic#102012) [esArchive] Persists updates for management/saved_objects/* (elastic#101992) skip flaky suite (elastic#101449) remove unnecessary hack (elastic#101909) [Exploratory View] Use human readable formats (elastic#101520) [Expressions] Refactor expression functions to use observables underneath (elastic#100409) [esArchives] Persist migrated Kibana archives (elastic#101950) [kbnArchiver] fix save to non-existent file (elastic#101974) [Enterprise Search] Add owner and description properties to kibana.json (elastic#101957) [DOCS] Fixes terminology in Stack Monitoring:Kibana alerts (elastic#101696) [Observability] [Cases] Cases in the observability app (elastic#101487) [Alerting][Docs] Combine rule creation and management pages (elastic#101498) temporarily disable build-buddy [Fleet] Fix fleet server collector in case settings are not set (elastic#101752) [Event Log] Populated rule.* ECS fields for alert events. (elastic#101132) [APM] Fleet support for merging input.config values with other nested properties in the policy input (elastic#101690) Add comments to some alerting plugin public API items (elastic#101551) [Alerting][Docs] Moving alerting setup to its own page (elastic#101323) remove uptime public API, it's not used. (elastic#101799) ...

dokmic force-pushed the feature/93016 branch from 47d6b35 to 3b09f0d Compare May 26, 2021 18:35

dokmic marked this pull request as ready for review May 27, 2021 07:52

dokmic requested review from a team as code owners May 27, 2021 07:52

dokmic added Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) release_note:skip Skip the PR/issue when compiling release notes review Team:AppServices v7.14.0 v8.0.0 labels May 27, 2021

dokmic requested a review from ppisljar May 27, 2021 07:54

ppisljar reviewed May 31, 2021

View reviewed changes

src/plugins/expressions/common/expression_functions/specs/map_column.ts Outdated Show resolved Hide resolved

ppisljar reviewed May 31, 2021

View reviewed changes

x-pack/plugins/canvas/canvas_plugin_src/functions/common/case.ts Outdated Show resolved Hide resolved

ppisljar reviewed May 31, 2021

View reviewed changes

flash1293 approved these changes Jun 2, 2021

View reviewed changes

poffdeluxe approved these changes Jun 7, 2021

View reviewed changes

ppisljar approved these changes Jun 8, 2021

View reviewed changes

dokmic force-pushed the feature/93016 branch 2 times, most recently from dc5e04c to 394fe55 Compare June 10, 2021 18:47

dokmic added 2 commits June 10, 2021 22:14

Refactor expression functions to use observables underneath

5e3f6eb

Update public API documentation

c893503

dokmic force-pushed the feature/93016 branch from 394fe55 to c893503 Compare June 10, 2021 20:15

dokmic merged commit 9bcae4d into elastic:master Jun 11, 2021

dokmic deleted the feature/93016 branch June 11, 2021 07:53

dokmic mentioned this pull request Jun 11, 2021

[7.x] [Expressions] Refactor expression functions to use observables underneath (#100409) #101988

Merged

dokmic added a commit that referenced this pull request Jun 11, 2021

[Expressions] Refactor expression functions to use observables undern…

f09704d

…eath (#100409) (#101988)

dokmic mentioned this pull request Aug 10, 2021

[Expressions] Add support of partial results to the switch expression function #108086

Merged

2 tasks

dokmic mentioned this pull request Sep 1, 2022

[Bug] unexpected behavior from Canvas expression switch function #131657

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Expressions] Refactor expression functions to use observables underneath #100409

[Expressions] Refactor expression functions to use observables underneath #100409

dokmic commented May 20, 2021 •

edited

Loading

elasticmachine commented May 27, 2021

ppisljar May 31, 2021

dokmic Jun 10, 2021

ppisljar May 31, 2021

dokmic Jun 10, 2021

ppisljar May 31, 2021

dokmic Jun 10, 2021

ppisljar May 31, 2021

dokmic Jun 10, 2021

ppisljar May 31, 2021

dokmic Jun 10, 2021

ppisljar May 31, 2021

dokmic Jun 10, 2021

flash1293 left a comment •

edited

Loading

poffdeluxe left a comment

ppisljar left a comment

kibanamachine commented Jun 10, 2021

[Expressions] Refactor expression functions to use observables underneath #100409

[Expressions] Refactor expression functions to use observables underneath #100409

Conversation

dokmic commented May 20, 2021 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented May 27, 2021

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flash1293 left a comment • edited Loading

Choose a reason for hiding this comment

poffdeluxe left a comment

Choose a reason for hiding this comment

ppisljar left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 10, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

dokmic commented May 20, 2021 •

edited

Loading

flash1293 left a comment •

edited

Loading