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

Tribe node: pass path.conf to inner tribe clients #16258

Merged
merged 1 commit into from Jan 27, 2016

Conversation

Projects
None yet
3 participants
@javanna
Member

javanna commented Jan 27, 2016

If we don't do this, and some path.conf is set when starting the tribe node, that path.conf will be ignored and the inner tribe clients will try to read elsewhere, where they most likely don't have permissions to read from.

Closes #16253

@javanna

This comment has been minimized.

Member

javanna commented Jan 27, 2016

The fix for this is easy. The problem is testing it. I spent a few hours trying to write a good test for this but I failed. I tried with an integration test to add to TribeIT, but I couldn't make it fail. I also looked into unit testing it but the TribeService would need to be refactored to allow for that, as it has way too much logic and dependencies on its constructor. Suggestions are welcome.

@javanna

This comment has been minimized.

Member

javanna commented Jan 27, 2016

one more thing: HunspellService is quite peculiar and it's the only component that would cause this type of problem I think, because it eagerly loads the dictionaries in each client node (not sure it's the best thing to do but it's another problem) when the node gets created. Same doesn't happen for synonyms for instance as loading happens when the token filter factory gets created. I thought also something like this could happen for logging, but there loading happens only once from a static block.

@s1monw

This comment has been minimized.

Contributor

s1monw commented Jan 27, 2016

LGTM - maybe @rjernst has an idea with test-fixtures

Tribe node: pass path.conf to inner tribe clients
If we don't do this, and some path.conf is set when starting the tribe node, that path.conf will be ignored and the inner tribe clients will try to read elsewhere, where they most likely don't have permissions to read from.

Closes #16253
Closes #16258
@javanna

This comment has been minimized.

Member

javanna commented Jan 27, 2016

I will get this in for now, as a followup we should definitely look at the tribe tests and rewrite them and add more coverage. Whatever we fix here is not properly tested at the moment because the tribe node is so hard to test...the least we can do is add some qa tests.

@clintongormley

This comment has been minimized.

Member

clintongormley commented Jan 27, 2016

++

@javanna javanna merged commit 533af17 into elastic:master Jan 27, 2016

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@clintongormley clintongormley added v2.1.2 and removed v2.1.3 labels Jan 27, 2016

javanna added a commit that referenced this pull request Jan 27, 2016

Tribe node: pass path.conf to inner tribe clients
If we don't do this, and some path.conf is set when starting the tribe node, that path.conf will be ignored and the inner tribe clients will try to read elsewhere, where they most likely don't have permissions to read from.

Closes #16253
Closes #16258

javanna added a commit that referenced this pull request Jan 27, 2016

Tribe node: pass path.conf to inner tribe clients
If we don't do this, and some path.conf is set when starting the tribe node, that path.conf will be ignored and the inner tribe clients will try to read elsewhere, where they most likely don't have permissions to read from.

Closes #16253
Closes #16258

javanna added a commit that referenced this pull request Jan 27, 2016

Tribe node: pass path.conf to inner tribe clients
If we don't do this, and some path.conf is set when starting the tribe node, that path.conf will be ignored and the inner tribe clients will try to read elsewhere, where they most likely don't have permissions to read from.

Closes #16253
Closes #16258

@clintongormley clintongormley added v2.2.0 and removed v2.2.1 labels Jan 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment