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

Migrate vega and graph configs to new platform #57011

Merged
merged 26 commits into from
Feb 14, 2020

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Feb 6, 2020

Depends on #56763

This PR gets rid of injected vars usage for Graph and vega config keys.
It also renames vega.* to vis_type_vega.* to be consistent with other visualization types. If the old config prefix is used, a warning is shown.

@flash1293 flash1293 added Feature:Vega Vega visualizations Feature:Graph Graph application feature v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:NP Migration v7.7.0 labels Feb 6, 2020
@flash1293 flash1293 added release_note:breaking and removed release_note:skip Skip the PR/issue when compiling release notes labels Feb 7, 2020
@flash1293
Copy link
Contributor Author

Jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed


Test Failures

Kibana Pipeline / kibana-intake-agent / Jest Integration Tests.packages/kbn-plugin-generator/integration_tests.running the plugin-generator via 'node scripts/generate_plugin.js plugin-name' with default config then running 'yarn test:browser' should exit 0

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches


Stack Trace

Error: Command failed with exit code 3: yarn test:browser
    at makeError (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/execa/lib/error.js:56:11)
    at handlePromise (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/execa/index.js:114:26)
    at process._tickCallback (internal/process/next_tick.js:68:7)

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

@flash1293 flash1293 mentioned this pull request Feb 10, 2020
5 tasks
@flash1293 flash1293 marked this pull request as ready for review February 10, 2020 18:13
@flash1293 flash1293 requested a review from a team February 10, 2020 18:13
@flash1293 flash1293 added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Feb 10, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@kertal kertal self-requested a review February 12, 2020 11:27
@kertal
Copy link
Member

kertal commented Feb 12, 2020

@elasticmachine merge upstream

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM 👍, tested locally in chrome and starting Kibana with
yarn start --vis_type_vega.enableExternalUrls=true

✅ works

yarn start --vega.enableExternalUrls=true

✅ works and found the expected output in the server startup logs:
log [16:16:42.561] [warning][config][deprecation] "vega.enableExternalUrls" is deprecated and has been replaced by "vis_type_vega.enableExternalUrls"

deprecations: ({ renameFromRoot }) => [
renameFromRoot('vega.enableExternalUrls', 'vis_type_vega.enableExternalUrls'),
renameFromRoot('vega.enabled', 'vis_type_vega.enabled'),
],
Copy link
Member

@kertal kertal Feb 12, 2020

Choose a reason for hiding this comment

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

This is nice ❤️, so for now it doesn't break

@flash1293
Copy link
Contributor Author

I have no idea why this is failing - the logs report an error within Joi:

13:21:18         │         313 modules
13:21:18         │          
13:21:18         │          ERROR in /dev/shm/workspace/kibana/node_modules/joi/lib/types/string/index.js
13:21:18         │          Module not found: Error: Can't resolve 'net' in '/dev/shm/workspace/kibana/node_modules/joi/lib/types/string'

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Graph Graph application feature Feature:NP Migration Feature:Vega Vega visualizations release_note:deprecation Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants