Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Vega] Shim new platform - vega_fn.js -> vega_fn.js , use ExpressionFunction #42582

Merged
merged 6 commits into from
Aug 9, 2019

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Aug 5, 2019

Summary

Part of #38247

What was done in this PR:

  1. vega_fn.js was renamed. New name vega_fn.ts
  2. vega_fn was refactored to use ExpressionFunction

Checklist

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

For maintainers

@alexwizp
Copy link
Contributor Author

alexwizp commented Aug 5, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@alexwizp alexwizp added v7.4.0 v8.0.0 Feature:Vega Vega visualizations Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes labels Aug 5, 2019
@alexwizp alexwizp marked this pull request as ready for review August 5, 2019 13:34
@alexwizp alexwizp requested a review from lukeelmers August 6, 2019 11:06
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with 1 small request

export const createVegaFn = (dependencies) => ({
name: 'vega',
const name = 'vega';
type Context = KibanaContext | null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukeelmers talked to @alexwizp
Were unclear about the meaning of the Context type, in the scope of ExpressionFunction?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lizozom @alexwizp Context is the output of one function that gets passed to the next. For example, in this expression:

esaggs aggConfig="blah blah" | vega spec="hi"

The value returned by the esaggs function and piped into the vega function is called "Context".

Each expression function defines which context types it accepts as context...

context: {
types: ['kibana_context', 'null'],
},

...And which context type it returns (Return interface) as type:

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alexwizp alexwizp merged commit dfd455d into elastic:master Aug 9, 2019
alexwizp added a commit to alexwizp/kibana that referenced this pull request Aug 9, 2019
…unction (elastic#42582)

* [Vega] Shim new platform - vega_fn.js -> vega_fn.js , use ExpressionFunction

* fix PR comments

* fix PR comments
alexwizp added a commit that referenced this pull request Aug 9, 2019
…unction (#42582) (#43011)

* [Vega] Shim new platform - vega_fn.js -> vega_fn.js , use ExpressionFunction

* fix PR comments

* fix PR comments
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 9, 2019
…p-metrics-selectall

* 'master' of github.com:elastic/kibana: (306 commits)
  [ML] Adding job overrides to the module setup endpoint (elastic#42946)
  [APM] Fix missing RUM url (elastic#42940)
  close socket timeouts without message (elastic#42456)
  Upgrade elastic/charts to 8.1.6 (elastic#42518)
  [ML] Delete old AngularJS data visualizer and refactor folders (elastic#42962)
  Add custom formatting for Date Nanos Format (elastic#42445)
  [Vega] Shim new platform - vega_fn.js -> vega_fn.js , use ExpressionFunction (elastic#42582)
  add socket.getPeerCertificate to KibanaRequest (elastic#42929)
  [Automation] ISTANBUL PRESET PATH is not working fine with constructor(private foo) (elastic#42683)
  [ML] Data frames: Updated stats structure. (elastic#42923)
  [Code] fixed the issue that the repository can not be deleted in some cases. (elastic#42841)
  [kbn-es] Support for passing regex value to ES (elastic#42651)
  Connect to Elasticsearch via SSL when starting kibana with `--ssl` (elastic#42840)
  Add Elasticsearch SSL support for integration tests (elastic#41765)
  Fix duplicate fetch in Visualize (elastic#41204)
  [DOCS] TSVB and Timelion clean up (elastic#42953)
  [Maps] [File upload] Fix maps geojson upload hanging on index step (elastic#42623)
  [APM] Use rounded bucket sizes for transaction distribution (elastic#42830)
  [yarn.lock] consistent resolve domain (elastic#42969)
  [Uptime] [Test] Repurpose unit test assertions to avoid flakiness (elastic#40650)
  ...
@alexwizp alexwizp deleted the vega_fn branch January 4, 2020 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration Feature:Vega Vega visualizations release_note:skip Skip the PR/issue when compiling release notes v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants