-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Configuration Form Editor in Dev UI is confusing - disable rewriting application.properties
#43229
Comments
/cc @cescoffier (devui), @phillip-kruger (devui), @radcortez (config) |
the editor has the value of allowing you to update the application.properties values. if you have env variable sets then that is effectively you saying "please override whatever app properties states" ... that is not something any config editor can change. we could maybe warn the user that this change possibly wont take effect as an env var is present? about suggestion on only allow some changes and fire cdi events - not sure what properties that would be because Quarkus does not in general support dynamic reload of the config as that brings even nastier bugs :) About iso/utf then that is a thing to change. I don't believe we support anything but UTF-8 anyways - but if smallrye readss your file fine with utf-8, so should quarkus. |
@ueberfuhr continous testing works as a restart afaik so not relevant there (IMO) Can you open the specific issues you found - as they looks relevant and fixable but trying to handle them all in one is going to be confusing. The solution isn't to disable the editor IMO :) |
Of course, I'll prepare PRs for them. |
Agree with @maxandersen I use the editor all the time with great success and have not run into any of the issues you mention above. So please do not take away editing! |
@melloware / @maxandersen - I have created a PR (a single one, because multiple PRs would depend on each other) with details about suggested changes. Please have a look to #43354 and test, if this matches your requirements. |
Description
Multiple irritations concerning the Configuration Form Editor in Dev UI, when we edit properties - and the editor stores the changes into the
application.properties
. TL;DR: I would suggest to disable re-writing properties changes to theapplication.properties
file.Encoding
SmallRye Config loads the
application.properties
with this code:try (BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream, UTF_8))) { properties.load(reader); }
(Source)
Quarkus'
ConfigurationProcessor
usesIn case of ISO-8859-1 encoded
application.properties
files, the first one works (except for some characters that are decoded to invalid chars), the second one throws an exception.Environment Variables
As described in the MicroProfile Config spec, properties are resolved from multiple config sources with a given priority. E.g. environment variables overwrite values in the
application.properties
. Changing the values in the Dev UI and writing them into theapplication.properties
won't work because they are still shadowed by the environment variables. Much worse: This changes the convention for environments that do not have such an environment variable!Profiles
Same with profiles. If we have a
%dev.<name>=<value>
property, it is re-written as<name>=<value>
, which won't work fordev
profile, but change default values for all other profiles!Formatting
If we use space chars in
application.properties
(variable = value
), then the entry will occur twice, because it is not found when re-writing the values.Maven Resource Filtering
If we use Maven resource filtering to replace
${}
expressions by Maven properties, the resolved values are shown in the Dev UI. If we change them, we will replace the${}
expression in the source. This is because of another difference between SmallRye Config and theConfigurationProcessor
: SmallRye loads fromtarget/classes/application.properties
, whereas theConfigurationProcessor
writes tosrc/main/resources/application.properties
.Hot Replacement
Of course, properties are loaded and injected only at startup, changes during runtime won't be processed by the components. For continuous testing, this might be helpful, so we could provide a CDI-event-based approach in the Dev UI, but this won't work for all properties. (and the developer should be aware of that - this might be unclear when providing an editor that allows changing values at runtime)
Implementation ideas
Because of all these confusing points, I'm not sure what added value the editor has (except for single cases like continuous testing). I would suggest to
application.properties
(this could be done by the developer manually)Of course, I could make a PR for this, but first I need the discussion about that.
The text was updated successfully, but these errors were encountered: