quarkusio / quarkus

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

Consistent naming scheme and semantics for enabled/active configuration properties #42244

Open yrodiere opened 2 months ago

yrodiere commented 2 months ago

Description

Following discussions on the mailing list, we've recently tried to use consistent conventions for active/enabled configuration properties in the datasource and Hibernate extensions.

The target behavior is as follows:

Worth noting that we're refining that behavior in #41810/#41929 so that startup will fail (with a detailed, actionable message) if any user bean gets injected with "inactive" beans. See https://github.com/quarkusio/quarkus/issues/36666#issuecomment-2162923149 for the motivation; in short it's about failing fast.

There are other extensions also exposing root "enabled"/"active" properties in their configuration, and many are following the same naming scheme and semantics (though they don't all offer the .active feature):

List of configuration properties that are following the same naming and semantics as datasource/Hibernate * [`quarkus.management.enabled`](https://quarkus.io/guides/all-config#quarkus-vertx-http_quarkus-management-enabled): Enables / Disables the usage of a separate interface/port to expose the management endpoints. If sets to true, the management endpoints will be exposed to a different HTTP server. This avoids exposing the management endpoints on a publicly available server. * [`quarkus.oauth2.enabled`](https://quarkus.io/guides/all-config#quarkus-elytron-security-oauth2_quarkus-oauth2-enabled): Determine if the OAuth2 extension is enabled. Enabled by default if you include the elytron-security-oauth2 dependency, so this would be used to disable it. * [`quarkus.flyway.enabled`](https://quarkus.io/guides/all-config#quarkus-flyway_quarkus-flyway-enabled): Whether Flyway is enabled during the build. * [`quarkus.info.enabled`](https://quarkus.io/guides/all-config#quarkus-info_quarkus-info-enabled): Whether the info endpoint will be enabled * [`quarkus.jacoco.enabled`](https://quarkus.io/guides/all-config#quarkus-jacoco_quarkus-jacoco-enabled): Whether or not the jacoco extension is enabled. * [`quarkus.micrometer.enabled`](https://quarkus.io/guides/all-config#quarkus-micrometer_quarkus-micrometer-enabled): Micrometer metrics support. * [`quarkus.oidc.enabled`](https://quarkus.io/guides/all-config#quarkus-oidc_quarkus-oidc-enabled): If the OIDC extension is enabled. * [`quarkus.oidc-client.enabled`](https://quarkus.io/guides/all-config#quarkus-oidc-client_quarkus-oidc-client-enabled): If the OIDC client extension is enabled. * [`quarkus.banner.enabled`](https://quarkus.io/guides/all-config#quarkus-core_quarkus-banner-enabled): Whether the banner will be displayed * [`quarkus.config-tracking.enabled`](https://quarkus.io/guides/all-config#quarkus-core_quarkus-config-tracking-enabled): Whether configuration dumping is enabled * [`quarkus.console.enabled`](https://quarkus.io/guides/all-config#quarkus-core_quarkus-console-enabled): If test results and status should be displayed in the console. * [`quarkus.devservices.enabled`](https://quarkus.io/guides/all-config#quarkus-core_quarkus-devservices-enabled): Global flag that can be used to disable all Dev Services. If this is set to false then Dev Services will not be used. * [`quarkus.observability.enabled`](https://quarkus.io/guides/all-config#quarkus-observability-devservices_quarkus-observability-enabled): If DevServices has been explicitly enabled or disabled. DevServices is generally enabled by default, unless there is an existing configuration present. * [`quarkus.scheduler.enabled`](https://quarkus.io/guides/all-config#quarkus-scheduler_quarkus-scheduler-enabled): If schedulers are enabled. * [`quarkus.webauthn.enabled`](https://quarkus.io/guides/all-config#quarkus-security-webauthn_quarkus-webauthn-enabled): If the WebAuthn extension is enabled. * [`quarkus.smallrye-jwt.enabled`](https://quarkus.io/guides/all-config#quarkus-smallrye-jwt_quarkus-smallrye-jwt-enabled): The MP-JWT configuration object * [`quarkus.virtual-threads.enabled`](https://quarkus.io/guides/all-config#quarkus-virtual-threads_quarkus-virtual-threads-enabled): A flag to explicitly disabled virtual threads, even if the JVM support them. In this case, methods annotated with @RunOnVirtualThread are executed on the worker thread pool.

However, others have taken a different route.

Some follow a slightly different naming scheme:

Some use an enabled root configuration property to disable the extension at runtime:

And finally some use an active root configuration property with a different meaning:

We even have pending PR (#42225) that would add a property quarkus.liquibase[."datasource-name"].active which takes effect at build-time... the reason being that quarkus.liquibase.enabled is already taken, and has effect on all datasources (but it's a runtime property!).

Let's discuss how to reconcile all this.


Note some extensions have an active/enabled property that is not at the root of their config, using it to enable/disable specific features (see below). I am not suggesting to change these in any way, though someone else might want to open a different issue to improve consistency there too; in particular metrics/tracing seems to be configured sometimes in the metrics/tracing extension, sometimes in the "integrated" (JDBC, ...) extension.

List of configuration properties that are not relevant to this issue: https://gist.github.com/yrodiere/16c8e429d0dd7b38bea3a97d6fdc86f5

Implementation ideas

I would suggest we:

I would tentatively suggest the following naming and semantics:

For the existing discrepancies, I would suggest the following.

Extensions using a slightly different naming scheme: align naming?

Extensions using an enabled root configuration property to disable the extension at runtime: rename to quarkus.*.active, and introduce build-time property quarkus.*.enabled?

Extensions with an active root configuration property with a different meaning: change the behavior to align on agreed-upon semantics?

quarkus-bot[bot] commented 2 months ago

/cc @geoand (kubernetes), @iocanel (kubernetes), @radcortez (config)

yrodiere commented 2 months ago

cc @gsmet

melloware commented 2 months ago

If we are voting I prefer "enabled" over "active" 😬

But I do see your point about consistency. All my extensions use "enabled" at the root because I followed other extensions examples.

yrodiere commented 2 months ago

If we are voting I prefer "enabled" over "active" 😬

Unfortunately we are not, at least not about "enabled vs. active". Both are necessary, because they address different needs:

  • quarkus.hibernate-orm.enabled=false (extension level) will disable the Hibernate extension at build time: it will be almost as if the extension was not in the class path, it won't do anything except declare its presence so that it can be displayed on app startup.

  • quarkus.hibernate-orm[."persistence-unit-name"].active=false (persistence unit level) will activate/deactivate a persistence unit at runtime: if inactive, the SessionFactory does not started on app startup and SessionFactory/Session/... beans cannot be accessed at runtime.

We can probably think about different name (though I'd rather not), but picking one over the other would mean removing a feature.

yrodiere commented 2 months ago

Another solution would be to change the convention:

I think it could makes sense given:

However, this would potentially be a breaking change in Hibernate extensions: .enabled would no longer be applied at build time, and would be applied only to the default persistence unit, potentially leading to build errors in places where the extension was disabled for a good reason.

xstefank commented 2 months ago

For the existing discrepancies, I would suggest the following.

Extensions using a slightly different naming scheme: align naming?

This will not work. quarkus.health.extensions.enabled is specifically saying that we don't include Quarkus provided checks in the responses, not disabling the whole extension. But I can maybe rename it to quarkus.health.extensions.included.

xstefank commented 2 months ago

@yrodiere @gsmet I'm also thinking that for the global enable/disable, if we are still printing the feature in the log on app startup, should it be something like smallrye-health (disabled) or smallrye-health (inactive)?

yrodiere commented 1 month ago

@yrodiere @gsmet I'm also thinking that for the global enable/disable, if we are still printing the feature in the log on app startup, should it be something like smallrye-health (disabled)

Makes sense to me.