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

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

Closed
raptus84 opened this issue Dec 7, 2023 · 9 comments
Assignees
Labels
area/devservices area/kafka kind/bug Something isn't working triage/out-of-date This issue/PR is no longer valid or relevant

Comments

@raptus84
Copy link

raptus84 commented Dec 7, 2023

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

@raptus84 raptus84 added the kind/bug Something isn't working label Dec 7, 2023
Copy link

quarkus-bot bot commented Dec 7, 2023

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

@raptus84 raptus84 changed the title quarkus.devservices.enabled property is ignored if user don't set a Kafka broker quarkus.devservices.enabled property is ignored if user doesn't set a Kafka broker Dec 7, 2023
@raptus84
Copy link
Author

raptus84 commented Dec 7, 2023

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
Copy link
Member

@raptus84, can you propose an adjustment?

@cescoffier cescoffier closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2024
@cescoffier cescoffier added the triage/out-of-date This issue/PR is no longer valid or relevant label Aug 21, 2024
@gsmet
Copy link
Member

gsmet commented Aug 21, 2024

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

@radcortez
Copy link
Member

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
Copy link
Member

gsmet commented Aug 21, 2024

Yeah, looks good, thanks.

@raptus84
Copy link
Author

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
Copy link
Member

radcortez commented Aug 21, 2024

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:

  • 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!

@raptus84
Copy link
Author

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 bandapplication-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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devservices area/kafka kind/bug Something isn't working triage/out-of-date This issue/PR is no longer valid or relevant
Projects
None yet
Development

No branches or pull requests

5 participants