-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Make boolean conversion strict #22200
Changes from 19 commits
b5d642b
e705d96
9fb7ea1
db3fd5e
c1b9df1
cbb3822
c09b301
665554d
1ee2db4
f7337ba
206adfa
fcfce6c
8de2eb4
6144f36
22aabf2
978f047
33af9f1
55b7e2b
0577acc
4c90110
f4a2f64
0f91b13
61f3912
bdc029e
fc8f39a
4523e55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1217,7 +1217,8 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti | |
* {@link #isIndexUsingShadowReplicas(org.elasticsearch.common.settings.Settings)}. | ||
*/ | ||
public static boolean isOnSharedFilesystem(Settings settings) { | ||
return settings.getAsBoolean(SETTING_SHARED_FILESYSTEM, isIndexUsingShadowReplicas(settings)); | ||
Version version = settings.getAsVersion(SETTING_VERSION_CREATED, Version.CURRENT); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure it is a good idea to have a default here? This shouldn't be missing in production, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I missed that one and will change it. |
||
return settings.getAsBooleanLenientForPreEs6Indices(version, SETTING_SHARED_FILESYSTEM, isIndexUsingShadowReplicas(settings)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe an additional arity version that didn't require the version and automatically looked it up from itself, that could save you some lookups (if you want) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use the |
||
} | ||
|
||
/** | ||
|
@@ -1226,7 +1227,8 @@ public static boolean isOnSharedFilesystem(Settings settings) { | |
* setting for this is <code>false</code>. | ||
*/ | ||
public static boolean isIndexUsingShadowReplicas(Settings settings) { | ||
return settings.getAsBoolean(SETTING_SHADOW_REPLICAS, false); | ||
Version version = settings.getAsVersion(SETTING_VERSION_CREATED, Version.CURRENT); | ||
return settings.getAsBooleanLenientForPreEs6Indices(version, SETTING_SHADOW_REPLICAS, false); | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it is worth having a version of
nodeBooleanValue
that tasks a string for the name of the field being converted. I know we have a few methods like that in other places.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that's a good idea. I'll need to check the call-sites.