-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
cleanup and standardize settings placeholders #11455
Comments
Would |
@jaymode Why do we need the |
The For clarification and background, I think the overall idea here is standardization of the placeholder naming. Looking at the replacePropertyPlaceholders method, we actually have two different ways of ignoring placeholders, the There is a reason for this mess. The prompt placeholders use the format If we standardize the naming, then we can clean this up and at the same time remove the |
Wondering if it wouldn't be easier (and more bwc) to use this then:
|
@clintongormley perhaps, it'll be more aligned with what we have today |
In elastic#10918, we introduced the prompt placeholders. These were had a different format than our existing placeholders. This changes the prompt placeholders to follow the format of the existing placeholders. Relates to elastic#11455
In elastic#10918, we introduced the prompt placeholders. These were had a different format than our existing placeholders. This changes the prompt placeholders to follow the format of the existing placeholders. Relates to elastic#11455
@clintongormley I think we still need to cleanup the code a little, (copied the first bullet from above):
We also didn't decide on the |
Prompting settings were removed in #27216, thus this issue is no longer relevant. |
In #10918, a new placeholder was added for prompting settings. We already allow placeholders to read values from the environment/system properties. During the review, @uboness noted that it would be good to standardize the naming and cleanup the code that prompts the place holders.
The new changes mentioned in the comments of the PR are:
Settings
and theInternalSettingsPreparer
. This logic is really internal and should not be exposed in the Settings.${env::foobar}
config.ignore_placeholders
:This will ignore all placeholder settings:
will only ignore
env
placeholders:will only ignore
prompt
placeholders:will ignore both the
env
andprompt
settings:The text was updated successfully, but these errors were encountered: