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

Don't bother setting the number of index replicas when creating kibana-int #5670

Closed

Conversation

eherot
Copy link
Contributor

@eherot eherot commented Dec 14, 2015

PR #3890 removed the hard-coding of number_of_replicas but the change appears to have been regressed by this monster commit: 8d2e881 (by @simianhacker) which deleted one of the two (???) create_kibana_index.js scripts and, unfortunately, not the one that #3890 modified.

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@rashidkpc
Copy link
Contributor

Before we can review this we'll need you to sign the CLA. Please check out CONTRIBUTING.md

@eherot
Copy link
Contributor Author

eherot commented Dec 15, 2015

Just signed the CLA. Now what?

@spalger
Copy link
Contributor

spalger commented Dec 15, 2015

Confirmed, the CLA has been signed

@spalger
Copy link
Contributor

spalger commented Dec 15, 2015

jenkins, test it

@@ -70,8 +70,6 @@ describe('plugins/elasticsearch', function () {
.to.have.property('settings');
expect(params.body.settings)
.to.have.property('number_of_shards', 1);
expect(params.body.settings)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we update this to make sure it doesn't set the number of replicas?

expect(params.body.settings).to.not.have.property('number_of_replicas');

@spalger
Copy link
Contributor

spalger commented Dec 18, 2015

jenkins, test it

@spalger spalger mentioned this pull request Dec 18, 2015
@rashidkpc
Copy link
Contributor

Failing tests

@rashidkpc
Copy link
Contributor

Closing for #5729

@rashidkpc rashidkpc closed this Dec 21, 2015
@eherot
Copy link
Contributor Author

eherot commented Dec 21, 2015

👍

rashidkpc added a commit that referenced this pull request Dec 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants