quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.73k stars 2.67k forks source link

quarkus.devservices.enabled property is ignored if user doesn't set a Kafka broker #37574

Closed raptus84 closed 2 months ago

raptus84 commented 10 months ago

Describe the bug

Hi, I'm using SmallRye Kafka connector, if I set quarkus.devservices.enabled= false or quarkus.kafka.devservices.enabled=false but I don't set the broker property, when application is starting Quarkus still uses DevServices ( Dev Services for Kafka started on log console). Is that a bug or a common behaviour?

Documentation reference:

Dev Services for Kafka is automatically enabled unless: quarkus.kafka.devservices.enabled is set to false the kafka.bootstrap.servers is configured

I understand that the first condition is the most important or these condition are in OR ?

Thank you for reply Regards R.

Expected behavior

Quarkus Kafka Dev Services are turned OFF

Actual behavior

Quarkus Kafka Dev Services are turned ON

How to Reproduce?

Latest Quarkus 3.6.0 or 3.5.3 versions and SmallRye Kafka connector. Remove kafka broker property and disable devservices.

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

quarkus-bot[bot] commented 10 months ago

/cc @alesj (kafka), @cescoffier (kafka), @geoand (devservices), @ozangunalp (kafka), @stuartwdouglas (devservices)

raptus84 commented 10 months ago

I think I found the issue: it is NOT clear (reading the documentation) that an application.properties file have to be always present with same profiled properties inside. I had only an application-dev.properties file configured and for some reason it always looked for a list of properties in application.properties file (that is not present), so it ignored my configuration and run DevServices mode. I think this behavior is NOT well presented into the documentation and I think it could also be some kind of bug in configurations retrieval.

cescoffier commented 10 months ago

@raptus84, can you propose an adjustment?

gsmet commented 2 months ago

@radcortez is the behavior mentioned in the comment above what we expect? If not, was it fixed?

radcortez commented 2 months ago

It is expected and documented here: https://quarkus.io/guides/config-reference#profile-aware-files

It is set as a warning in the appropriate section. I'm not sure how to make it clearer.

gsmet commented 2 months ago

Yeah, looks good, thanks.

raptus84 commented 2 months ago

It is expected and documented here: https://quarkus.io/guides/config-reference#profile-aware-files

It is set as a warning in the appropriate section. I'm not sure how to make it clearer.

The doc reference doesn't explain that there should always be a default application.properties file. When I first read the docs I understood that I could create an application- only. I didn't expect that application-<> works overriding the default one. It is not very clear and a bit useless (in my opinion) having this behaviour. If I want to keep one property file only for each environment, why should I have to keep the default properties file with the same properties inside of it? I think it's weird.

radcortez commented 2 months ago

The doc reference doesn't explain that there should always be a default application.properties file. When I first read the docs I understood that I could create an application- only. I didn't expect that application-<> works overriding the default one. It is not very clear and a bit useless (in my opinion) having this behaviour. If I want to keep one property file only for each environment, why should I have to keep the default properties file with the same properties inside of it? I think it's weird.

You only require an application.properties file. It doesn't need to have properties in it.

We require the main file to be present to ensure a consistent ordering when loading the files. Consider the scenarion

a.jar
- application.properties
- application-dev.properties
b.jar
- application-dev.properties
c.jar
- application.properties
- application-dev.properties

When you query the Classloader for multiple resources, the order is not guaranteed. For example, you may get application.properties from a and b and application-dev.properties from c, b, a. So, how would you be able to assemble the final configuration? Profile properties must be higher in ordinality, so if we follow the order given by the classloader, we will get:

The problem here is that application.properties should follow the same order of application-dev.properties, c first and then a to pair them together.

We could compare paths and ensure the exact ordering, but where would we put application-dev.properties from b? We would need to move it to the bottom of the list because Config will read and load application.properties to configure itself (the profile property can be supplied in application.properties). These scenarios become even more complex when you mix in multi-profiles and config_ordinal override. Yes, it is implementable, but it would require non-trivial rules, so we opted for a simple single rule:

An application.properties file must exist to act as a marker of the location to load profile-aware files. In this way, we query the Classloader for these files first and then query the profile-aware files from each of their locations. The order is still subject to how the Classloader returns the resources, but we are not relying on multiple calls to figure out the order later.

Of course, I'm biased regarding many of the implementation details and documentation. Feel free to propose an alternate implementation or an enhancement to the documentation to clarify. Thanks!

raptus84 commented 2 months ago

The doc reference doesn't explain that there should always be a default application.properties file. When I first read the docs I understood that I could create an application- only. I didn't expect that application-<> works overriding the default one. It is not very clear and a bit useless (in my opinion) having this behaviour. If I want to keep one property file only for each environment, why should I have to keep the default properties file with the same properties inside of it? I think it's weird.

You only require an application.properties file. It doesn't need to have properties in it.

We require the main file to be present to ensure a consistent ordering when loading the files. Consider the scenarion

a.jar
- application.properties
- application-dev.properties
b.jar
- application-dev.properties
c.jar
- application.properties
- application-dev.properties

When you query the Classloader for multiple resources, the order is not guaranteed. For example, you may get application.properties from a' andbandapplication-dev.propertiesfromc,b,a`. So, how would you be able to assemble the final configuration? Profile properties must be higher in ordinality, so if we follow the order given by the classloader, we will get:

  • application-dev.properties (from c)
  • application-dev.properties (from b)
  • application-dev.properties (from a)
  • application.properties (from a)
  • application.properties (from c)

The problem here is that application.properties should follow the same order of application-dev.properties, c first and then a to pair them together.

We could compare paths and ensure the exact ordering, but where would we put application-dev.properties from b? We would need to move it to the bottom of the list because Config will read and load application.properties to configure itself (the profile property can be supplied in application.properties). These scenarios become even more complex when you mix in multi-profiles and config_ordinal override. Yes, it is implementable, but it would require non-trivial rules, so we opted for a simple single rule:

An application.properties file must exist to act as a marker of the location to load profile-aware files. In this way, we query the Classloader for these files first and then query the profile-aware files from each of their locations. The order is still subject to how the Classloader returns the resources, but we are not relying on multiple calls to figure out the order later.

Of course, I'm biased regarding many of the implementation details and documentation. Feel free to propose an alternate implementation or an enhancement to the documentation to clarify. Thanks!

Ok thats clear now and of course it was not my case. If there is this level of complexity my suggestion is to write down this answer as a note also into the documentation. I lost a couple of days understanding that it was always running in dev mode because I set the properties only in my application-dev.properties and not in my application.properties. It was a bit unclear for me. thanks