Repository navigation
variable support for interpreter - #54788
Conversation
|
Pinging @elastic/kibana-app-arch (Team:AppArch) |
2164002 to
97677aa
Compare
streamich
left a comment
There was a problem hiding this comment.
Code LGTM, given the value falsy check, see below.
Also, would be nice to add unit tests for both functions.
| }, | ||
| async fn(context, args, handlers) { | ||
| const variables: Record<string, any> = handlers.variables; | ||
| if (!variables[args.name]) { |
There was a problem hiding this comment.
In case value is falsy, like 0:
| if (!variables[args.name]) { | |
| if (variables[args.name] === undefined) { |
or
| if (!variables[args.name]) { | |
| if (!variables.hasOwnProperty(args.name)) { |
or maybe we could use a Map
| if (!variables[args.name]) { | |
| if (!variables.has(args.name)) { |
| async fn(context, args, handlers) { | ||
| const variables: Record<string, any> = handlers.variables; | ||
| if (!variables[args.name]) { | ||
| throw new Error(`Variable "${args.name}" does not exist`); |
There was a problem hiding this comment.
Shall we throw here? Or maybe we could return undefined?
There was a problem hiding this comment.
you are right, thanks!
For setting:
For using:
|
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
|
@ppisljar how do we test this pr? Thanks! |
|
this is currently not exposed in any way to the user, so its can't really be tested and should not have any implications for users. |
Summary
As already defined in the types, interpreter handlers can contain the list of variables. This PR adds the implementation needed for that to work:
varfunction is added to the function registry, which has a single argumentnameand it would read that variable from thehandlers.variablesand return it.var_set(looking for better name suggestions) accepts anameand optionally avalueand would update the variable with this value. If value is not provided context will be used as variable value.var_setfunction returns the same context it received (so it does not affect the following functions, unless they are using a variable that was updated)when running the expression the app can decide if it wants local variables (dashboard would pass separate instance of variable object to each embeddable) or global variables shared among embeddables (same instance of variables object would be passed to all embeddables)
Closes #46908
Example
using variables provided by the app executing the expression:
esaggs indexPattern={var indexPattern} aggs={....} | ...reusing context within another expression
esaggs .... | var_set storedContext | pie ....var storedContext | table ...Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers