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

[plugin installer] check multiple default paths for config #7440

Merged
merged 2 commits into from
Jun 15, 2016

Conversation

jbudz
Copy link
Member

@jbudz jbudz commented Jun 13, 2016

With #7275 packages install their config to /etc/kibana/kibana.yml by default. This updates the plugin installer to check multiple default paths, and adds the package path to the list.

@jbudz
Copy link
Member Author

jbudz commented Jun 13, 2016

jenkins, test it

];

let defaultPath = paths[0];
paths.some(configPath => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't using _.find() make more sense than _.some() here? They both iterate through a collection and stop once a condition is met, but... some is used to answer a question about the state of a collection; "Are there some elements in the collection that meet these conditions?" Where find is used to return an element in a collection; "Give me the first element of a collection that meets these conditions"

@BigFunger BigFunger assigned jbudz and unassigned BigFunger Jun 14, 2016
@jbudz jbudz assigned BigFunger and LeeDr and unassigned jbudz and LeeDr Jun 14, 2016
@LeeDr
Copy link

LeeDr commented Jun 14, 2016

Are we checking multiple paths for kibana.yml to cover both the cases of package installation and archive installation?

  • for a package install we create and use /etc/kibana/kibana.yml
  • for users who just unzip or untar the archive fromRoot('config/kibana.yml')?

Or are we trying to handle an upgrade case without having to move the previous version kibana.yml?

@jbudz
Copy link
Member Author

jbudz commented Jun 14, 2016

@LeeDr checking multiple paths for package and archive installation.

@epixa
Copy link
Contributor

epixa commented Jun 15, 2016

This seems like it needs to be a blocker since #7308 is merged. Otherwise plugin install will fail on package installs, right?

Edit: fixed PR link

@epixa epixa added the blocker label Jun 15, 2016
@epixa
Copy link
Contributor

epixa commented Jun 15, 2016

If #7308 ends up getting backported, make sure this does as well.

@jbudz
Copy link
Member Author

jbudz commented Jun 15, 2016

Correct, it'll fail unless -c /etc/kibana/kibana.yml is specified

@BigFunger
Copy link
Contributor

LGTM

@BigFunger BigFunger assigned jbudz and unassigned BigFunger Jun 15, 2016
@jbudz jbudz merged commit a167253 into elastic:master Jun 15, 2016
@epixa epixa added v5.0.0 and removed v5.0.0 labels Jun 28, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
[plugin installer] check multiple default paths for config

Former-commit-id: a167253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants