Quartz - Defer driver discovery to runtime in order to provide more flexibility to users

[manovotn]

Resolves #48545

[manovotn]

Deliberately kept a draft for now as I have yet to verify this with a case similar to what was reported in the issue.
Our own testing does not contain a dual DB setup.

EDIT: I have tested this with the reproducer provided by the user (see this comment) and it passes.

[manovotn]

I am aware that this PR does not move the build time config property into runtime config because I am frankly not sure if that would be breaking - technically it will have the same name but anything operating with the build time config might be affected by it. We could also temporarily duplicate them in both and deprecate one.

Anyhow, @mkouba thoughts on the approach in this PR and the config change? :)

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit b7c72bb.

Failing Jobs

Status	Name	Step	Failures	Logs	Raw logs	Build scan
✖	Native Tests - Virtual Thread - Main	Build	⚠️ Check →	Logs	Raw logs	🔍

You can consult the Develocity build scans.

[gsmet]

Can't you register for reflection only the dialects that could be triggered by the available JdbcDataSourceBuildItem?

[manovotn]

Not sure I follow.
The use case here is to delay choosing the datasource until runtime, so how would I know which one to register?

[gsmet]

A JdbcDataSourceBuildItem has a dbKind that is fixed at build time. So from them, you can deduce the list of databases that you have to support.

E.g. you have two datasources, one MySQL, one PostgreSQL, the dbKind of the datasources is defined at build so you know which ones might be useful, you don't need to register for reflection all of them.

Note: we also need to support when the kind is something we don't know, but I think we currently push something generic in this case? And you can hardcode it anyway.

[manovotn]

Ah, right I get it now.
+1, that makes sense

[mkouba]

I can't say that I completely understand the use case 🤔.

If a user sets the quarkus.quartz.datasource then we should IMO still try to use this DS and fail at build time if not available. If the quarkus.quartz.datasource is not set (i.e. the default DS should be used) then we could defer the driver discovery to runtime. Am I missing something?

[manovotn]

I can't say that I completely understand the use case 🤔.

Take a look at the discussion on the issue - #48545 (comment)
The reproducer is also a nice depiction of it.

If a user sets the quarkus.quartz.datasource then we should IMO still try to use this DS and fail at build time if not available. If the quarkus.quartz.datasource is not set (i.e. the default DS should be used) then we could defer the driver discovery to runtime. Am I missing something?

Maybe I don't fully understand but it seems that wouldn't work here.
In this case, the reproducer contains ConfigSourceInterceptor - an interceptor for config value of quarkus.quartz.datasource. And it returns either postgresql or mssql based on which datasource is active.
And the activity of datasource is determined at runtime.
So whatever information about quarkus.quartz.datasource you read during build time might not reflect reality at runtime.

Of course it is a matter of how much runtime flexibility we want to give it. This PR is just my attempt at grasping the use case and coming up with something that would make the reproducer pass.

[mkouba]

I can't say that I completely understand the use case 🤔.

Take a look at the discussion on the issue - #48545 (comment) The reproducer is also a nice depiction of it.

If a user sets the quarkus.quartz.datasource then we should IMO still try to use this DS and fail at build time if not available. If the quarkus.quartz.datasource is not set (i.e. the default DS should be used) then we could defer the driver discovery to runtime. Am I missing something?

Maybe I don't fully understand but it seems that wouldn't work here. In this case, the reproducer contains ConfigSourceInterceptor - an interceptor for config value of quarkus.quartz.datasource. And it returns either postgresql or mssql based on which datasource is active. And the activity of datasource is determined at runtime. So whatever information about quarkus.quartz.datasource you read during build time might not reflect reality at runtime.

Of course it is a matter of how much runtime flexibility we want to give it. This PR is just my attempt at grasping the use case and coming up with something that would make the reproducer pass.

My point is - if a user sets quarkus.quartz.datasource=foo then the foo DS should be used no matter what. Currently, it's a build time property so we can validate the existence of DS etc. If no DS is set then we fall back to the default DS. Which is also fine.

Now, it seems that the user needs to define multiple data sources, deactivate some and select one for Quartz at runtime. This means we would need to relax the validation at build time. Which is 👎.

Also I think it's breaking because the behavior's changed. And the ConfigSourceInterceptor completely ignores the value set by the user which is IMO weird.

The question is: could we introduce two new config properties that would be used if no DS is selected at build time (quarkus.quartz.datasource is not used). I.e. something like quarkus.quartz.datasource.none-selected-strategy (buildtime) with values default - use the default DS (current behavior), defer - select the DS at runtime. And then quarkus.quartz.datasource.deferred-name (runtime), only used if quarkus.quartz.datasource.none-selected-strategy=defer, that could be set by the ConfigSourceInterceptor or even build item. WDYM?

[manovotn]

And the ConfigSourceInterceptor completely ignores the value set by the user which is IMO weird.

Yes but that's not what the reproducer does. In it, there is no value set by user and only the interceptor exists so that it can give you the config value "dynamically", so to speak.

Also I think it's breaking because the behavior's changed.

Breaking in what way? Currently you'd only not get as fast failure if you misconfigured it but otherwise it should appear to behave equally on the outside?

Now, it seems that the user needs to define multiple data sources, deactivate some and select one for Quartz at runtime. This means we would need to relax the validation at build time. Which is 👎.

It is not the first time we hear about the need to have multiple DS, it's just that Quartz bit didn't have to deal with it yet.
I agree that having as much as possible done in build time is the way to go of course.

The question is: could we introduce two new config properties that would be used if no DS is selected at build time (quarkus.quartz.datasource is not used). I.e. something like quarkus.quartz.datasource.none-selected-strategy (buildtime) with values default - use the default DS (current behavior), defer - select the DS at runtime. And then quarkus.quartz.datasource.deferred-name (runtime), only used if quarkus.quartz.datasource.none-selected-strategy=defer, that could be set by the ConfigSourceInterceptor or even build item. WDYM?

Hm, interesting idea. However, in such a case, wouldn't it be easier to keep quarkus.quartz.datasource as is and just add something like quarkus.quartz.datasource.defer-resolution (default false of course)? The latter would just control whether we attempt to read and resolve the datasource property in build or runtime.
As far as I am concerned, the less configuration knobs, the better :)

[mkouba]

However, in such a case, wouldn't it be easier to keep quarkus.quartz.datasource as is as is...

The idea is to keep quarkus.quartz.datasource as is, which means a build time config property.

The latter would just control whether we attempt to read and resolve the datasource property in build or runtime.

I don't think it's a good idea. You can't treat a build config property as runtime and vice versa. They are processed differently. I.e. there is no way to tell Quarkus that quarkus.quartz.datasource is a build time property when quarkus.quartz.datasource.defer-resolution=false.