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

Enhance SmallRyeConfig handling #41994

Open
zakkak opened this issue Jul 19, 2024 · 5 comments
Open

Enhance SmallRyeConfig handling #41994

zakkak opened this issue Jul 19, 2024 · 5 comments

Comments

@zakkak
Copy link
Contributor

zakkak commented Jul 19, 2024

Description

As discussed in https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Best.20way.20to.20get.20config.20profile.20in.20a.20BuildStep.3F

Quarkus currently processes configuration in the deployment stage and generates a bytecode recorded class handling all the necessary configuration thus no longer relying (in theory at least) on the actual configuration files, except for dev mode maybe where Quarkus needs to detect changes to the configuration files and update the configuration.

In native-mode this bytecode recorded class is the only source of configuration (as the configuration files don't get embed in the native executable). In JVM-mode though it looks like that configuration files may still be available. This discrepancy enables JVM-mode to change profiles on the fly, which while not forbidden in native-mode it essentially has no effect as the bytecode recorded class is based on a specific profile and a warning is thrown to let the user know.

This leads to the following questions:

  1. What has higher priority the recorded bytecode or the configuration files?
  2. What happens when we change the profile in JVM-mode? Does the bytecode recorded configuration get replaced by the runtime loaded configuration?
  3. If the configuration files are reachable, isn't SmallRye loading them? Essentially beating the purpose of the bytecode recorded class in the first place?

Implementation ideas

@radcortez suggested disabling SmallRye discovery of configuration files for non-dev, non-test modes. Adding a note that "the discovery class, also looks for external files in a config folder sitting in the working directory, we don't have a way to fine tune. either it is enabled for classpath and folder discovery or none, we would need to enhance it to support it properly"

@zakkak suggested stripping the configuration files after deployment is done

@zakkak zakkak added the kind/enhancement New feature or request label Jul 19, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 19, 2024

You added a link to a Zulip discussion, please make sure the description of the issue is comprehensive and doesn't require accessing Zulip

This message is automatically generated by a bot.

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 19, 2024

/cc @Ladicek (smallrye), @jmartisk (smallrye), @phillip-kruger (smallrye), @radcortez (config,smallrye)

@radcortez
Copy link
Member

  • What has higher priority the recorded bytecode or the configuration files?

The configuration files have priority. Usually, this is not an issue. The configuration works by sources, meaning the file is a source, and the recorded bytecode is another source. Both are loaded and a configuration property is queried to all sources in order.

The recorded configuration bytecode comes from the configuration files. In JVM mode, both file and recorded are availalbe, meaning that on configuration lookup, the providing source will be the file. In native mode, since the files are not available, the providing source is the recorded bytecode (which is mostly a copy of the files).

  • What happens when we change the profile in JVM-mode? Does the bytecode recorded configuration get replaced by the runtime loaded configuration?

No, it is still there, and it is always loaded due to how the configuration works (everything is a source, sources are loaded in order, and queries are performed in that order until a value is found).

  • If the configuration files are reachable, isn't SmallRye loading them? Essentially beating the purpose of the bytecode recorded class in the first place?

Yes, we could probably improve this piece in JVM mode. I think we shouldn't be removing the files from the deployment, because we don't know if the user / consumer is using them from something else. We would need to implement a few enhancements to selectively disable the classpath discovery of the configuration files.

@zakkak
Copy link
Contributor Author

zakkak commented Jul 22, 2024

Do you recall what was the initial motivation of using recorded bytecode as a source? Was it for better performance, reduced native executable size or something else?

@radcortez
Copy link
Member

All of them :)

While most applications probably only ship one file, we look for application.properties and microprofile-config.properties in the classpath, so you can end up with multiple ones from different jars.

zakkak added a commit to zakkak/quarkus that referenced this issue Jul 24, 2024
`SmallryeConfigBuilder` tries to access properties files from various
sources, despite them not being available at run-time nor supported in
native-mode, see quarkusio#41994

This patch also avoids getting a `MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Related to quarkusio#41994 and
quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 24, 2024
`SmallryeConfigBuilder` tries to access properties files from various
sources, despite them not being available at run-time nor supported in
native-mode, see quarkusio#41994

This patch also avoids getting a `MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Related to quarkusio#41994 and
quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 25, 2024
`SmallryeConfigBuilder` tries to access properties files from various
sources, despite them not being available at run-time nor supported in
native-mode, see quarkusio#41994

This patch also avoids getting a `MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Related to quarkusio#41994 and
quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 26, 2024
`SmallryeConfigBuilder` tries to access properties files from various
sources, despite them not being available at run-time nor supported in
native-mode, see quarkusio#41994

This patch also avoids getting a `MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Related to quarkusio#41994 and
quarkusio#41995
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants