Skip to content
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

Automatically remove trailing spaces in the application.properties file #19956

Closed
kshpak opened this issue Sep 7, 2021 · 6 comments · Fixed by #19958
Closed

Automatically remove trailing spaces in the application.properties file #19956

kshpak opened this issue Sep 7, 2021 · 6 comments · Fixed by #19958
Labels
area/config kind/enhancement New feature or request
Milestone

Comments

@kshpak
Copy link
Contributor

kshpak commented Sep 7, 2021

Description

Trailing spaces after property in the application.properties file can cause unwanted errors.
Example:

  1. generate an application with the quarkus-openshift extension
  2. specify quarkus.application.name=myservice (with a trailing space)
  3. mvn clean package
  4. Build error
Build step io.quarkus.container.image.deployment.ContainerImageProcessor#publishImageInfo threw an exception: java.lang.IllegalArgumentException: The supplied combination of container-image group 'kshpak' and name 'myservice ' is invalid

Is there a reason to leave trimming on developers?

Implementation ideas

No response

@kshpak kshpak added the kind/enhancement New feature or request label Sep 7, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 7, 2021

/cc @geoand, @iocanel

@geoand
Copy link
Contributor

geoand commented Sep 7, 2021

Good question.

@radcortez does MP Config say anything about this?

@gsmet
Copy link
Member

gsmet commented Sep 7, 2021

We don't do that globally but we apply a converter locally for the properties for which it makes sense. See TrimmedStringConverter.

@geoand
Copy link
Contributor

geoand commented Sep 7, 2021

Thanks for that tip @gsmet. I somehow managed to miss it :).

I'll add that for the container-image properties.

@radcortez
Copy link
Member

Good question.

@radcortez does MP Config say anything about this?

Nothing, because it follows the behavior of properties files (trailing spaces are read).

I understand that this may be inconvenient for some users, but on the other hand, we cannot blindly remove all trailing spaces for every configuration, because it means we may be changing the user configuration and we don't know if the user wants to have the space there or not.

geoand added a commit to geoand/quarkus that referenced this issue Sep 7, 2021
geoand added a commit that referenced this issue Sep 7, 2021
Trim container-image configuration values
@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Sep 7, 2021
@mmDonuts
Copy link

Just a heads up that this change changes the following behavior: if setting the group to empty using a trailing space, you need to replace it with an empty quoted string

@gsmet gsmet modified the milestones: 2.3.0.CR1, 2.2.4.Final Nov 30, 2021
gsmet pushed a commit to gsmet/quarkus that referenced this issue Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants