Code Monkey home page Code Monkey logo

Comments (43)

dmlloyd avatar dmlloyd commented on June 12, 2024 1

The build time analog would then be application-build.conf then I guess?

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024 1

Also lost from the initial discussion: the ability to read a configuration off the filesystem. Either the filesystem variant could be layered above the in-JAR one as a separate ConfigSource, or alternatively perhaps the ConfigSource for each could have a search path where the filesystem instance would replace the in-JAR instance.

from quarkus.

cescoffier avatar cescoffier commented on June 12, 2024 1

Using $PWD is not recommended as it would prohibit the usage of volume to mount the file

from quarkus.

gsmet avatar gsmet commented on June 12, 2024 1

I think what's @cescoffier was suggesting is that we do:

  • jar:/application.properties (or jar:/config/applications.properties)
  • $PWD/config/application.properties

So that he can mount the $PWD/config directory as a volume.

from quarkus.

mkouba avatar mkouba commented on June 12, 2024

I think we should support both, microprofile-config.properties and protean.properties. From MP Config POV it should be just one more ConfigSource. @kenfinnigan am I right?

from quarkus.

gsmet avatar gsmet commented on June 12, 2024

Hmmm, protean.properties or shamrock.properties? For now, all our properties are prefixed with shamrock.

I think I would prefer a more neutral application.properties as @cescoffier suggested.

from quarkus.

cescoffier avatar cescoffier commented on June 12, 2024

application.properties makes migration from springboot "easier"

from quarkus.

mkouba avatar mkouba commented on June 12, 2024

application.properties works for me.

from quarkus.

kenfinnigan avatar kenfinnigan commented on June 12, 2024

+1 to application.properties and yes it should just be another ConfigSource.

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

It would be nice to kill the .properties, maybe use e.g. .conf instead?

from quarkus.

FroMage avatar FroMage commented on June 12, 2024

protean.conf or application.conf both work for me.

from quarkus.

gsmet avatar gsmet commented on June 12, 2024

I think we should go with application.properties as the Spring Boot argument is compelling.

I really don't see any value in introducing a .conf suffix which is far less common in Java.

Moving this to first-public-release as it would be a good idea to set the configuration file name in stone before the first public release.

from quarkus.

mkouba avatar mkouba commented on June 12, 2024

I think we should go with application.properties as the Spring Boot argument is compelling.

Why don't we just support all (application.properties, microprofile-config.properties - this one is imho required by MicroProfile and MP users are used to it, protean.properties) and only mention application.properties in docs?

I really don't see any value in introducing a .conf suffix which is far less common in Java.

+1

from quarkus.

gsmet avatar gsmet commented on June 12, 2024

Ah, yes, sorry, it was unclear, it was a reply for David and his .conf proposal.

Agreed we could support application.properties and microprofile-config.properties. It's simple to do but I think it will probably be in the way of what David is working on.

from quarkus.

emmanuelbernard avatar emmanuelbernard commented on June 12, 2024

Proposal is to use application{-profile}.properties at the root. It's at the root because it is not a JAR specific info (which META-INF is about).

from quarkus.

emmanuelbernard avatar emmanuelbernard commented on June 12, 2024

More conversation on what we want

Had a discussion around Eclipse MicroProfile Config and what it means for Protean.
The SmallRye doc is empty so I assume it semantic and impls == the spec
and nothing more.

It did bother us that the config file was in META-INF which means that such property is specific to each archive and not shared for the whole app by design. So I read the spec and here is my proposal for Protean

  1. we create a new ConfigSource with priority 210 (i.e. above the
    META-INF one) and that one looks at - /application[-profile].properties
  2. we create a new ConfigSource with priority 200 (i.e. above the
    META-INF one) and that one looks at - /application.properties Note that 1 and 2 are separated sources as MP Config handles the overriding of properties well?
  3. we create a new ConfigSource with priority 350 (right level?) i.e. below System.getenv and that read data from ConfigMap.

Of course it should not be explained as ConfigSource to users as it a bit too much details.

3 is kube specific
1 and 2 is about clarifying that we have a stack wide configuration that overrides specific artifact ones.

This is 100% spec compliant.

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

We originally talked about using key prefixes (e.g. %dev.shamrock.foo=bar) for profile key values (possibly with the idea of adding per-profile defaults in the future, so that dev maps to development mode etc.). Do we still want to do that? Would it be instead of the [-profile] infix? Or in addition?

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

In fact I'd take it one step farther and say we should not package a configuration in the JAR, because it would contain only defaults and the defaults are already baked in to the bytecode so it gives us nothing. The user would be able to generate an example configuration anyway.

Users could decide to package a different configuration into the JAR if they wanted to though, in the case where they don't want a filesystem config file and they're not running in native mode.

from quarkus.

FroMage avatar FroMage commented on June 12, 2024

We originally talked about using key prefixes (e.g. %dev.shamrock.foo=bar) for profile key values (possibly with the idea of adding per-profile defaults in the future, so that dev maps to development mode etc.). Do we still want to do that? Would it be instead of the [-profile] infix? Or in addition?

We still want to have profiles, yes. And they're really separate from the mode.

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

We originally talked about using key prefixes (e.g. %dev.shamrock.foo=bar) for profile key values (possibly with the idea of adding per-profile defaults in the future, so that dev maps to development mode etc.). Do we still want to do that? Would it be instead of the [-profile] infix? Or in addition?

We still want to have profiles, yes. And they're really separate from the mode.

That wasn't the question at all! :)

What I'm asking is, how do we want to do profiles? Do we want to do it by key (e.g. %profileName.shamrock.foo=...) or by having multiple files with the [-profile] in the name?

from quarkus.

FroMage avatar FroMage commented on June 12, 2024

It looks like we want both options, but I'm not sure if the keys in [-profile] files need to be prefixed with %profileName or not.

The alternative is to support @include directives and let users do %stefProfile@include=application-stefProfile.conf in the main config file.

from quarkus.

emmanuelbernard avatar emmanuelbernard commented on June 12, 2024

Hold on, let me clarify the options as I understand them:

  1. application.properties containing %profilename.shamrock.
  2. application[-%profilename] contains profile specific properties and override the ones in application.properties
  3. in application.properties one adds %profilename@include=some-profile-specific.properties

I think I like the simplicity of 1.

from quarkus.

aloubyansky avatar aloubyansky commented on June 12, 2024

I think I prefer the simplicity of 2 :) In one you'd have to keep pasting/typing %profilename, which is not that convenient in itself and even more so in case you want to create another profile by copying the existing one if that makes sense.
The advantage of 1 as I see it is single source.

from quarkus.

FroMage avatar FroMage commented on June 12, 2024

The big disadvantage of multiple files is that you have to either duplicate every setting if they're disjoint, or look in two files for a global view of the settings if they're merged.

There's a big UX advantage in having them all in the same file as in 1.

from quarkus.

aloubyansky avatar aloubyansky commented on June 12, 2024

AFAIU, the intention of duplication would be to override, so not really a duplication? The max depth of the hierarchy would be 2? So for a complete config for each profile you'd end up with 2 files? The main config and the profile-specific overrides?

from quarkus.

emmanuelbernard avatar emmanuelbernard commented on June 12, 2024

Actually the potential hierarchy is more than 2 since youhave all the other ConfigSource (-D, envvar, ConfigMap). To me that would also push for 1. since we would not have a profile diamond mess just on application.properties and not the other sources.

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

Another advantage of one file is that we can theoretically generate defaults for multiple profiles and have them all be side by side in the same file.

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

Also recall how important it was early in the requirements gathering process that we have one config file: I don't think that importance is diminished just because we have come up with an implementation option that contradicts it.

from quarkus.

aloubyansky avatar aloubyansky commented on June 12, 2024

There are arguments from exactly the UX perspective which are in favor of 2 that can't be denied. I just want to mention them. I am not going to claim they outweigh everything else but again they can't be overlooked neither.
I think if the amount of properties is fairly small then 1 is hands down the best option. If statistically this is going to be the case then that's it.
However with the number of properties growing up 2 becomes more attractive to me. Just think about managing profile configs (assuming there is no specialized tooling which makes it all actually irrelevant to the end user).

  1. Create a new profile. Having a significant list of properties, you have no choice but copy-paste, prefix property names and adjust values. As a side-effect that'll also enlarge the whole file significantly. With the second option you simply copy the base file and adjust values.
  2. Removing a profile is trivial.
  3. Temporarily disabling a profile (by renaming the file to something that's not picked up by the build or runtime) is also trivial instead of commenting it out in a large file.
  4. The point of having all the properties in one file helps seeing the big picture is becoming less evident in case your terminal (or window) is actually displaying 5% (or less) of the whole config.
  5. Locating profile config. In a large file you'd have to search for the profile name prefix and then hopefully the properties are arranged in a group. If they are not and appear mixed with other properties that becomes a mess. With the second option it's easy to locate and impossible to mess it up.
  6. If the hierarchy of configs is already in place regardless of this choice then combining all the profiles in one file is not going to make it flat.

Also you can think about it this way: the hierarchy is essentially in there regardless of the choice (meaning the notion of the base and profile overriding the base is essentially is hierarchy and it does not depend on whether it's going to be 1 or 2). The choice between 1 and 2 is about expressing that hierarchy.

from quarkus.

emmanuelbernard avatar emmanuelbernard commented on June 12, 2024

For the sake of moving on, I'll use what I call the lead hammer and say we go for a single file handing the profile configs.
Most of your arguments are valid @aloubyansky but the scenario of hairy config file is going to be very uncommon. And we can add the two files option later.

from quarkus.

aloubyansky avatar aloubyansky commented on June 12, 2024

Sure, that's fine.

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

Right now I'm assuming "/application.properties" in the app class path with an additional config source of "${PWD}/application.properties" as a slightly higher priority. If anyone wants to object, now's the time.

from quarkus.

Sanne avatar Sanne commented on June 12, 2024

I'd suggest using ${PWD}/config/application.properties, or even better to allow passing the full config path to the main as application parameter - @dmlloyd mentioned on the call that this isn't possible currently but I've not understood if that's a limitation which could be easily overcome - don't we generate our own main?

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

Using $PWD is not recommended as it would prohibit the usage of volume to mount the file

This has been said multiple times, but I don't see why. You don't have to provide it.

I'd suggest using ${PWD}/config/application.properties, or even better to allow passing the full config path to the main as application parameter - @dmlloyd mentioned on the call that this isn't possible currently but I've not understood if that's a limitation which could be easily overcome - don't we generate our own main?

Nothing can be easily done because our class loading is a dumpster fire so getting config to initialize in the right order in main and in test and in dev is very difficult as it is. Let's just stick to the bare requirements for now and see where that takes us.

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

The initial PR is up @cescoffier so if you have an answer, now's the time. Specifically - is @Sanne's suggestion OK? Or can we not care for the first release? Or something else?

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

PR #1110 partially solves this issue. As discussed in chat, it does not introduce build.properties yet. Also since application.properties is only read during run time, existing ITs which have a mix of build and run time properties in microprofile-config.properties will have to continue using that file until build.properties is introduced (the build.properties file would include the build-time configuration keys plus default value overrides for the application at run time, so that applications can start up with minimal/no configuration).

from quarkus.

gsmet avatar gsmet commented on June 12, 2024

@dmlloyd I think we should discuss this on the list. I wasn't aware you planned to introduce 2 different configuration files.

The jar application.properties is included in the jar at build time so I fail to see why it is only read at runtime.

I would have had only one config file in the jar, called application.properties.

The filesystem config/application.properties would be read at runtime.

Or do I miss something?

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

Build properties are not recognized at run time and would result in a warning for being unrecognized, so they should be separated.

In many cases the application can be started up with no configuration, since the defaults would have been set up during build. This was one of the requirements we discussed in Toronto. We also discussed the requirement of having a reasonable configless startup whenever possible.

So the typical flow would be to provide the build configuration with defaults, and then optionally give a run time configuration to perform what few overrides are desired in the given environment.

It's true that application.properties would be included at build time if you make it so. But I'm not sure there's a lot of utility there TBH. Normally you'd want this file to either be empty or to only have a couple of properties to wire in specifics for a given environment.

from quarkus.

gsmet avatar gsmet commented on June 12, 2024

Build properties are not recognized at run time and would result in a warning for being unrecognized, so they should be separated.

This I can see.

Build properties are not recognized at run time and would result in a warning for being unrecognized, so they should be separated.

What I don't find useful is having 2 different configuration files in the jar.

In the jar, I would have only one config file called application.properties containing build time and runtime configuration (be they specific to a profile).

All the runtime properties present in the jar are default values, be they specific to a profile.

Is there any specific value to have 2 separate config files in the jar? I might be missing something.

I think we should ask others what they think about it as I'm pretty sure a lot of us didn't expect having 2 different config files in the jar.

from quarkus.

dmlloyd avatar dmlloyd commented on June 12, 2024

Is there any specific value to have 2 separate config files in the jar?

No, you do not want 2 separate configs in the JAR. In fact you probably don't even want one config in the JAR.

The build configuration is an input to the augmentation process. But it doesn't have to be in the original JAR; in fact there's really no value to putting it there, other than that just being a convenient place to put it that the plugin can pick up without additional configuration.

The build configuration can contain three kinds of properties:

  • Properties which affect the build (aka BUILD_TIME)
  • Properties which affect the build but are also visible to the run time (aka BUILD_AND_RUN_TIME_FIXED)
  • Properties which are not visible to the build but specify default values for run time properties (aka RUN_TIME)

During the build process, these properties get parsed out and assigned to their various destinies: the BUILD_TIME and BUILD_AND_RUN_TIME_FIXED properties get loaded into their respective objects for injection into build steps; the BUILD_AND_RUN_TIME_FIXED properties get loaded into a properties file (with all variables fully expanded) that is read during static initialization to initialize the run time views of those build time properties; and the RUN_TIME properties get loaded into a properties file that will be added as a low-priority configuration source when the run time config is established during the main startup. Any properties which start with quarkus. and do not belong to one of these three groups will cause a warning message to be logged that the property is not recognized and will be ignored.

After this process, build.properties is no longer used and would be discarded.

The application.properties only comes into play at run time. The user would normally have set all their desired default values in the build configuration. The user could add the application.properties to their JAR at build time but it's a bit pointless: if they had values they wanted to set in their application, why not just set them in the build properties to begin with? The run time application.properties would only contain overrides for certain specific situations, and in many cases wouldn't even be necessary because the user could always set their "flexible" properties to have variable expressions for values which can be resolved from system properties or environment variables. Importantly, build properties are not recognized at this point and would result in warnings being printed.

Now maybe this conflicts with the mental picture of having an application.properties bundled in the JAR - but really this is a pretty pointless thing to do for reasons I've mentioned. Maybe what everyone has been thinking of application.properties is really just build.properties. Maybe what we really need to discuss is whether everyone is in fact happy with these names. I don't really care too much what we call these things, but we do need these concepts to exist, and we really do need these two files to have distinct names, otherwise people will wonder "why was this file accepted at build time, but rejected at run time?" and that sort of thing.

Does that make sense?

from quarkus.

jmesnil avatar jmesnil commented on June 12, 2024
  1. we create a new ConfigSource with priority 350 (right level?) i.e. below System.getenv and that read data from ConfigMap.

@emmanuelbernard We have something that reads from ConfigMap (based on its file system representation) in smallrye-config at https://github.com/smallrye/smallrye-config/blob/master/implementation/src/main/java/io/smallrye/config/DirConfigSource.java

We use it in EAP to read configuration from OpenShift (as explained in http://jmesnil.net/weblog/2017/06/16/eclipse-microprofile-config-in-openshift/).

It might fit your use case (and it needs better documentation in smallrye-config in any case)

from quarkus.

emmanuelbernard avatar emmanuelbernard commented on June 12, 2024

@jmesnil can you open a issue with that info. I don't know if we have it working in quarkus today. Nobody could answer. We definitely need it working and documented when we go describe the kubernetes approach.

from quarkus.

kenfinnigan avatar kenfinnigan commented on June 12, 2024

@gsmet good to push documentation changes

from quarkus.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.