Remove config prompting for secrets and text #27216
Conversation
…cli-remove_prompt_terminal
…cli-remove_prompt_terminal
|
I chatted w/ @rjernst and i will be finding a way to add deprecation logging to this (where this is the 6.x version of this). I dont know how early this is in bootstrap but i suspect its before logging is set up, but ill find a way to hack it in. |
| */ | ||
| private static void finalizeSettings(Settings.Builder output, Terminal terminal) { | ||
| private static void checkSettingsForTerminalDeprecation(final Settings.Builder output) throws SettingsException { | ||
| for (String setting : output.keys()) { |
There was a problem hiding this comment.
Let's add an assertion here the major property of the current version is not 8; then, when we bump the version to 8.0.0 on master we know that we can remove this code.
There was a problem hiding this comment.
dude, nice. Thats smart. Will do.
There's a way to hack it in: pass whether or not the terminal was used to set a setting up through the environment and then in bootstrap extract that from the environment and log a deprecation message if it was used. Although, I do wonder if this is overkill (is emitting a message on the terminal enough?)? |
I think it would be nice to have in the deprecation log. It can cause peoples clusters to break and they wont really know it until spinning them up if they pay less attn to the standard log / terminal. That said, is there any prior art to what we do in these infra-y breaking changes? If we do not normally do this, then its perfectly valid not to add it for this. Im still leaning toward seeing it in the deprecation logs tho. |
There are times where we simply can not have the fancy deprecation logging and rely on the migration docs. I think I want to see how ugly and invasive the hack is before rendering judgement here. My concern with a hack that lives in 6.x only is it making backporting other changes more difficult. |
|
Ill give it a whirl and see how ugly it is. |
| */ | ||
| private static void checkSettingsForTerminalDeprecation(final Settings.Builder output) throws SettingsException { | ||
| // This method to be removed in 8.0.0, as it was deprecated in 6.0 and removed in 7.0 | ||
| if (Version.CURRENT.toString().startsWith("8")) { |
There was a problem hiding this comment.
How about Version.CURRENT.major == 8? And I think maybe we should only make this an assertion?
There was a problem hiding this comment.
Thx, i wasnt aware exactly how to do it
…cli-remove_prompt_terminal
…cli-remove_prompt_terminal
…cli-remove_prompt_terminal
* master: (31 commits) [TEST] Fix `GeoShapeQueryTests#testPointsOnly` failure Transition transport apis to use void listeners (#27440) AwaitsFix GeoShapeQueryTests#testPointsOnly #27454 Bump test version after backport Ensure nested documents have consistent version and seq_ids (#27455) Tests: Add Fedora-27 to packaging tests Delete some seemingly unused exceptions (#27439) #26800: Fix docs rendering Remove config prompting for secrets and text (#27216) Move the CLI into its own subproject (#27114) Correct usage of "an" to "a" in getting started docs Avoid NPE when getting build information Removes BWC snapshot status handler used in 6.x (#27443) Remove manual tracking of registered channels (#27445) Remove parameters on HandshakeResponseHandler (#27444) [GEO] fix pointsOnly bug for MULTIPOINT Standardize underscore requirements in parameters (#27414) peanut butter hamburgers Log primary-replica resync failures Uses TransportMasterNodeAction to update shard snapshot status (#27165) ...
This commit removes the ability to use ${prompt.secret} and
${prompt.text} as valid config settings. Secure settings has obsoleted
the need for this, and it cleans up some of the code in Bootstrap.