Skip to content

Replace group map settings with affix setting #26819

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

Merged
merged 7 commits into from
Sep 30, 2017

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Sep 28, 2017

We use group settings historically instead of using a prefix setting which
is more restrictive and type safe. The majority of the usecases needs to access
a key, value map based on the leave node of the setting ie. the setting
index.tag.* might be used to tag an index with index.tag.test=42 and
index.tag.staging=12 which then would be turned into a {"test": 42, "staging": 12}
map. The group settings would always use Settings#getAsMap which is loosing type information
and uses internal respesentation of the settings. Using prefix settings allows now to
access such a method typesafe and natively.

We use group settings historically instead of using a prefix setting which
is more restrictive and type safe. The majority of the usecases needs to access
a key, value map based on the _leave node_ of the setting ie. the setting
`index.tag.*` might be used to tag an index with `index.tag.test=42` and
`index.tag.staging=12` which then would be turned into a `{"test": 42, "staging": 12}`
map. The group settings would always use `Settings#getAsMap` which is loosing type information
and uses internal respesentation of the settings. Using prefix settings allows now to
access such a method typesafe and natively.
@s1monw s1monw requested a review from rjernst September 28, 2017 16:53
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -577,6 +577,16 @@ public void testAffixKeySetting() {
assertFalse(listAffixSetting.match("foo"));
}

public void testAffixAsMap() {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test for what happens if there are further subkeys? eg foo.bar.baz.deep

@s1monw s1monw merged commit 7b8d036 into elastic:master Sep 30, 2017
s1monw added a commit that referenced this pull request Sep 30, 2017
We use group settings historically instead of using a prefix setting which is
more restrictive and type safe. The majority of the usecases needs to access a
key, value map based on the _leave node_ of the setting ie. the setting
`index.tag.*` might be used to tag an index with `index.tag.test=42` and
`index.tag.staging=12` which then would be turned into a `{"test": 42,
"staging": 12}` map. The group settings would always use `Settings#getAsMap`
which is loosing type information and uses internal representation of the
settings. Using prefix settings allows now to access such a method type-safe and
natively.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Oct 1, 2017
* xdcr:
  Maybe die before trying to log cause
  Log cause when a write and flush fails
  Die if write listener fails due to fatal error
  RecoveryIT.testHistoryUUIDIsGenerated should reduce unassigned shards delay instead of ensure green.
  Replace group map settings with affix setting (elastic#26819)
  Fix references to vm.max_map_count in Docker docs
  Add more instructions about jar hell (elastic#26837)
  Forbid negative values for index.unassigned.node_left.delayed_timeout (elastic#26828)
  Nitpicking typos in comments (elastic#26831)
  MetaData Builder doesn't properly prevent an alias with the same name as an index (elastic#26804)
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.

3 participants