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

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

Open
yrodiere opened this issue Jul 31, 2024 · 8 comments

Comments

@yrodiere
Copy link
Member

yrodiere commented Jul 31, 2024

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:

  • 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.
  • (and similarly for quarkus.hibernate-envers and (quarkus.hibernate-search-*`)
  • quarkus.datasource[."datasource-name"].active (datasource level) will activate/deactivate a datasource at runtime: if inactive, the connection pool does not gets started on app startup and the Datasource bean cannot be accessed at runtime.

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 #36666 (comment) 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

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:

  • quarkus.flyway.active: Flag to activate/deactivate Flyway for a specific datasource at runtime.
    NOTE: The documentation (previous sentence) is unclear, but this actually only disables "on-startup" behavior for Flyway. You can still retrieve the Flyway bean and trigger schema changes, so it's not really "deactivated".

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:

  • agree on naming/semantics once and for all
  • create a PR to align existing core extensions
  • create a PR to include these rules in a guide or some documentation aimed at extension writers

I would tentatively suggest the following naming and semantics:

  • quarkus.myextension[.name-for-datasource-or-persistence-unit-or-client-or-whatever].enabled: disables a datasource/persistence unit/client (if that makes sense) or whole extension (otherwise) at build time. It will be as if the extension didn't exist
  • quarkus.myextension[.name-for-datasource-or-persistence-unit-or-client-or-whatever].active: deactivates a datasource/persistence unit/client (if that makes sense) or whole extension (otherwise) at build time
  • And possibly, to avoid ambiguity between disabling an extension and e.g. disabling just the default persistence unit, a built-in feature in Quarkus (no need to implement anything in extensions) to disable extensions that are in the classpath. This could be either quarkus.myextension.enable-extension, or more likely some more arcane property like quarkus.extensions."extension-id".enabled.

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?

  • quarkus.cache.enabled => rename to quarkus.cache.active, and introduce quarkus.cache.enabled for build-time only?
  • quarkus.liquibase.enabled => rename to quarkus.liquibase.active, and introduce quarkus.liquibase.enabled for build-time only?
  • quarkus.jfr.enabled => rename to quarkus.jfr.active, and introduce quarkus.jfr.enabled for build-time only?
  • quarkus.kubernetes-config.enabled => rename to quarkus.kubernetes-config.active, and introduce quarkus.kubernetes-config.enabled for build-time only?
  • quarkus.load-shedding.enabled => rename to quarkus.load-shedding.active, and introduce quarkus.load-shedding.enabled for build-time only?

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

  • quarkus.flyway.active: disable both flyway's on-start behavior and access to beans as soon as active is false?
@yrodiere yrodiere added the kind/enhancement New feature or request label Jul 31, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 31, 2024

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

@yrodiere
Copy link
Member Author

cc @gsmet

@melloware
Copy link
Contributor

melloware commented Jul 31, 2024

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

yrodiere commented Aug 1, 2024

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

yrodiere commented Aug 1, 2024

Another solution would be to change the convention:

  • use .build-enabled for build-time enablement -- or if we stick to extension level, quarkus.myextension.enable-extension or quarkus.extensions."extension-id".enabled
  • use .enabled for runtime enablement
  • keep .active where it exists, but deprecate it

I think it could makes sense given:

  • the keyword "active" is kind of exotic
  • I suspect the need for disabling extensions at at build-time is rather rare; you'd generally just remove the extension from your depedencies.
  • people generally don't think about the build-time/runtime split when configuring Quarkus, so they probably expect enabled to just work at runtime.

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

xstefank commented Aug 8, 2024

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

xstefank commented Aug 8, 2024

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

yrodiere commented Aug 26, 2024

@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.

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

3 participants