Create separate startup scripts for development and production by jbudz · Pull Request #13806 · elastic/kibana

jbudz · 2017-08-31T16:57:56Z

This creates a different set of startup scripts for development and production. Development scripts are not included in builds.

There's a few reasons behind this:

We added --no-warnings during crunch time to hide unhandled promise rejections. In node 4 this was the default,, and we didn't have time to track these down with node 6. This removes the --no-warnings flag in development.
This changes development to only use the system's global node. It's unlikely anyone was making use of the node folder in development and it's possible this could get in the way
This changes production to only use the packaged node. Currently both version 4 and version 8 won't work. We've also had problems with this and homebrew in the past.
Removing NODE_ENV=production in development

Closes #5673.

Development mode is now started with node scripts/kibana, and for plugins node scripts/kibana-plugin. CLI args are the same.

jbudz · 2017-08-31T17:09:11Z

Another possible benefit that hasn't been included in this PR is we can stop shipping $dev code, and set it as a default in exec. (like --base-path)

epixa · 2017-08-31T17:55:14Z

❤️

If we're going to do this though, what do you think about removing the bin entirely for development and just going through node scripts/dev or something like that? Then we don't need to maintain separate binaries for nix/windows, and when we split out all the dev garbage from cli, we could easily "wrap" the cli call from within the dev script.

/cc @spalger since he was just talking about separating out dev stuff from the CLI.

spalger · 2017-08-31T23:01:25Z

+1 @epixa

./scripts/dev.js could probably just point to ./src/cli, and since node scripts/dev calls node directly it removes --no-warnings, makes it obvious that it's using system node, and how to use --inspect and friends.

jbudz · 2017-08-31T23:29:48Z

Works for me, I'll make updates.

jbudz · 2017-09-01T16:56:30Z

Getting there - we'll need grunt-run to use exec or fork, I don't think spawn can start another node process. I need to switch gears for a few, will come back to this.

spalger · 2017-09-02T07:34:20Z

I don't think spawn can start another node process

Sure it can, it's just another executable. Try using process.execPath if it's not resolving node to the right path.

jbudz · 2017-09-21T21:24:36Z

jenkins, test it

spalger · 2017-09-22T17:39:09Z

Are you sure this change is alright?

We are going through scripts/kibana.js as opposed to bin/kibana in development, so this should be correct

Whoops I forgot to comment on this. @spalger were you concerned about a different node version being used? I believe we're using the bin script anyways, the node build task isn't copied until after optimize.

I'll give this a double check.

This is producing builds that don't have optimized assets because it's running the optimization in the repo, not the build output.

spalger · 2017-10-05T17:53:20Z

I'm removing release_note:breaking because this doesn't impact releases at all, right?

jbudz · 2017-10-05T17:57:34Z

I added it in case someone was using a global node installed. It's not supported but technically would work and probably throw someone off.

tylersmalley

LGTM

tylersmalley · 2017-10-31T15:40:54Z

@spalger - mind taking another look at this?

spalger · 2017-10-31T21:27:18Z

This is producing builds that don't have optimized assets because it's running the optimization in the repo, not the build output.

w33ble · 2017-11-20T20:00:26Z

@elastic/plugin-helpers 8.0.0 published 🎉

@jbudz you should update the dependency here, and then it'll be safe to merge this PR.

jbudz · 2017-11-20T21:36:08Z

Thanks @w33ble!

@tylersmalley I opened https://github.com/elastic/kibana/pull/15066/files to address kibana-keystore - I separated it mostly because this is all green. Pending any red CI I'm going to get this merged in the morning so I can keep an eye on things.

spalger · 2017-11-29T20:37:03Z

This script should probably be using snake_case

epixa · 2017-11-29T21:30:56Z

@spalger When we adopted the snake_case thing for files, we did say that there may be some directories where it makes sense to use a different convention, and that we'd treat those case by case. Since these file names are essentially CLI commands for us, it's possible this is one of those cases.

spalger · 2017-11-30T17:19:38Z

possible, but I think it's ideal to think of the scripts/ as just javascript, and there are other files in that directory that are already using snake case

epixa · 2017-11-30T19:36:38Z

@spalger 👍

jbudz · 2017-11-30T20:07:15Z

I opened #15318 and assigned myself. I'll need to make upstream changes in the plugin-helpers and generator repos.

w33ble · 2017-11-30T20:33:26Z

@jbudz If I understand this correctly, you're planning to move scripts/kibana-plugin.js to scripts/kibana_plugin.js (likewise the keystore).

The plugin helpers are just using scripts/kibana.js, so no change there. And the template just uses the plugin helpers...

jbudz · 2017-11-30T20:57:58Z

Ah yep you're right, brain fart. That makes life easier.

schersh · 2019-02-12T16:30:35Z

@jbudz Should this be included in the Breaking Changes page for 7.0?
It appeared in the 7.0.0-alpha1 release notes, but I don't see a corresponding note in /migrate_7_0.asciidoc.

jbudz · 2019-02-12T17:07:29Z

I think we're okay, it has never been an approved or advertised workflow. I knew one case on the AUR repository that was swapping it out. We have it documented in a few GitHub issues which should be findable, I'll drop the breaking changes label.

jbudz · 2019-02-12T17:11:10Z

I spoke to soon, it's here https://github.com/elastic/kibana/blob/7.0/docs/migration/migrate_7_0.asciidoc#removed-support-for-running-kibana-with-a-global-nodejs-installation.

schersh · 2019-02-12T20:19:40Z

Thank you - sorry for the extra ping!

…ic#13806) * Use separate startup scripts for development and production * build kibana directly * [build] Use downloaded node when pre-optimizing * clearer variable name * Add breaking changes docs

jbudz added release_note:breaking v7.0.0 Team:Operations Kibana-Operations Team review labels Aug 31, 2017

jbudz added updates_needed and removed review labels Aug 31, 2017

jbudz force-pushed the dev/bin-kibana branch 3 times, most recently from e2086ba to 87f9ff1 Compare September 21, 2017 17:35

jbudz force-pushed the dev/bin-kibana branch from df691d2 to 5890360 Compare September 21, 2017 22:27

jbudz added review and removed updates_needed labels Sep 21, 2017

spalger reviewed Sep 22, 2017

View reviewed changes

spalger removed the release_note:breaking label Oct 5, 2017

jbudz force-pushed the dev/bin-kibana branch from bc94ca1 to 5efccf1 Compare October 5, 2017 17:59

spalger added the release_note:breaking label Oct 5, 2017

tylersmalley approved these changes Oct 10, 2017

View reviewed changes

spalger suggested changes Oct 31, 2017

View reviewed changes

jbudz added updates_needed and removed review labels Nov 1, 2017

tylersmalley approved these changes Nov 17, 2017

View reviewed changes

jbudz force-pushed the dev/bin-kibana branch from 4e59963 to 595b3f0 Compare November 20, 2017 21:19

jbudz added 5 commits November 20, 2017 15:31

Use separate startup scripts for development and production

e4bb525

build kibana directly

b77f1dd

[build] Use downloaded node when pre-optimizing

f61a3e8

clearer variable name

cfb69f1

Add breaking changes docs

6f24929

jbudz force-pushed the dev/bin-kibana branch from 595b3f0 to 6f24929 Compare November 20, 2017 21:31

jbudz mentioned this pull request Nov 20, 2017

Remove node fallback from kibana-keystore #15066

Merged

jbudz mentioned this pull request Nov 21, 2017

Bump plugin helpers to 8.0.1 elastic/template-kibana-plugin#12

Merged

jbudz added the release_note:breaking label Nov 22, 2017

jbudz merged commit 218986c into elastic:master Nov 22, 2017

jbudz mentioned this pull request Nov 30, 2017

Snake case scripts/kibana-{plugin,keystore} #15318

Closed

jbudz removed the release_note:breaking label Feb 12, 2019

watson mentioned this pull request Aug 20, 2019

Remove duplication in dev startup script #43568

Closed

5 of 7 tasks

Conversation

jbudz commented Aug 31, 2017 • edited by rhoboat Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbudz commented Aug 31, 2017

Uh oh!

epixa commented Aug 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spalger commented Aug 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbudz commented Aug 31, 2017

Uh oh!

jbudz commented Sep 1, 2017

Uh oh!

spalger commented Sep 2, 2017

Uh oh!

jbudz commented Sep 21, 2017

Uh oh!

spalger Sep 22, 2017

Choose a reason for hiding this comment

Uh oh!

tylersmalley Oct 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbudz Oct 11, 2017

Choose a reason for hiding this comment

Uh oh!

spalger Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

spalger commented Oct 5, 2017

Uh oh!

jbudz commented Oct 5, 2017

Uh oh!

tylersmalley left a comment

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Oct 31, 2017

Uh oh!

spalger Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

w33ble commented Nov 20, 2017

Uh oh!

jbudz commented Nov 20, 2017

Uh oh!

spalger commented Nov 29, 2017

Uh oh!

epixa commented Nov 29, 2017

Uh oh!

spalger commented Nov 30, 2017

Uh oh!

epixa commented Nov 30, 2017

Uh oh!

jbudz commented Nov 30, 2017

Uh oh!

w33ble commented Nov 30, 2017

Uh oh!

jbudz commented Nov 30, 2017

Uh oh!

schersh commented Feb 12, 2019

Uh oh!

jbudz commented Feb 12, 2019

Uh oh!

jbudz commented Feb 12, 2019

Uh oh!

schersh commented Feb 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jbudz commented Aug 31, 2017 •

edited by rhoboat

Loading

epixa commented Aug 31, 2017 •

edited

Loading

spalger commented Aug 31, 2017 •

edited

Loading

tylersmalley Oct 10, 2017 •

edited

Loading