support conditional bean

Dieken commented 3 years ago

Description

I want to enable or disable Kafka consumer based on a runtime property, currently @IfBuildProperty doesn't support runtime property.

https://docs.micronaut.io/latest/guide/#conditionalBeans

This @Requires annotation looks good:

@ApplicationScoped
@Requires(property="my.consumer.enabled", value="true")
public class MyConsumer {
    @Incoming("topic")
    public void consume(Xxx x) {
    }
}

Implementation ideas

Implementation something like Micronaut @Requires.

quarkus-bot[bot] commented 3 years ago

/cc @manovotn, @mkouba

Dieken commented 3 years ago

Is there any ETA for this feature? I have a service to store some data which is input by service API and Kafka consumer, the service itself is very simple, less than 1000 LOC, I wouldn't like to separate common code to a common jar and maintain pom dependency.

Currently I have to use quarkus.arc.exclude-types build time configuration or @IfBuildProperty to build two docker images from same code base, this is rather annoying, I would like single docker image and a runtime property to enable or disable the Kafka consumer.

And if I have more features to enable or disable, I hate building dozens of docker images from same code base :-(

mkouba commented 3 years ago

The Quarkus DI solution is purely build-time oriented, i.e. the bean discovery is performed and all dependencies are resolved at build time. Therefore, we cannot support this kind of feature.

In many cases, a producer method that returns a concrete instance based on a rutime config property can be used as a workaround. Unfortunately, that's not the case of the snippet above - AFAIK @Incoming can only be defined on a class bean.

Dieken commented 3 years ago

@mkouba @manovotn

Maybe Quarkus DI can support it:

at build time, Quarkus DI thinks all discovered beans are required, generates some meta data to represent the bean dependency DAG
at run time, Quarkus DI evaluates @Requires annotation and instantiates beans starting from the root of the dependency DAG
- if the bean has @Requires and it evaluates to false, then this bean and its dependent beans are skipped.

Is this feasible? I really need runtime conditional bean, I can't bear building dozens of jars to disable some beans.

manovotn commented 3 years ago

This cannot really be implemented in a CDI-way becuase CDI requires you to perform resolution and validation upfront. With the requirement you have, any build-time approach is lost because it would have to be re-done in runtime. Not to mention that it kills further optimizations such as removal of unused beans (see https://quarkus.io/guides/cdi-reference#remove_unused_beans).

Maybe you could approach it differently and instead have those beans produced via producers? Or provide some more code showing the scenario you are trying to cover and maybe we can find a different way to resolve this in a CDI-friendly manner.

mkouba commented 3 years ago

The problem is that we would have to defer the validation of all affected beans and dependencies to the runtime. Moreover, we would have to give up on many optimizations we do at build time (e.g. quarkus detects unused beans so that it can skip the metadata generation unless it's really needed).

Imagine there's a bean Foo which is annotated with @Requires(property="my.consumer.enabled", value="true"). This would mean that validation of any bean that has Bar in its dependency tree must be deferred to the runtime because the availability of Bar is only known at runtime.

Last but not least, the CDI container is started during the native image pre-boot phase, i.e. too soon to reflect the runtime config properties. In other words, it would never work for native images.

I do understand your use case but I can't think of a feasible solution. Maybe SR messaging could introduce a way to ignore some channels based on runtime config? If I'm not mistaken the SR channels are built at runtime so in theory, a user could provide some logic to activate/deactivate them CC @Ladicek @cescoffier

Dieken commented 3 years ago

@manovotn @mkouba

I showed a code snippet in the issue description, that's the scenario I encounter, probably I didn't describe clearly my proposal, let me use some pseudo-code to explain further.

I mean @Requires annotation is ignored at build time, so build time validation is still done normally. Those absolutely unused beans can still be removed at build time, those possibly unused beans can't be removed.

About the runtime logic, it may like this:

If Quarkus generates DI code like google dagger:

objA = new ClassA();
objB = new ClassB(objA);

becomes:

if (required(objA_id))  {     // required() method checks system properties for @Required annotation.
objA = new ClassA();
if (required(objB_id)) {   //  if objA is already disabled, objB is also disabled, no need for validation again
   objB = new ClassB(objA);
}
}

If Quarkus uses Class.newInstance(), the logic is similiar, just replace "new()" with "newInstance()".

Last but not least, the CDI container is started during the native image pre-boot phase, i.e. too soon to reflect the runtime config properties. In other words, it would never work for native images.

My knowledge to Quarkus is not enough to understand this, currently I still stick to non-native image, it starts quickly enough and builds very fast. But certainly I hope my proposal with some enhancement can work for native image too.

About the native image pre-boot phase, I feel @Requires won't be used for too many beans, so pre-boot phase still can optimize many beans, and the user of @Requires annotation must be aware of the trade-off.

Maybe SR messaging could introduce a way to ignore some channels based on runtime config?

Thanks, this can resolve my problem for this scenario above, would like to have that feature, but it's still better to have a general solution to disable beans at runtime.

mkouba commented 3 years ago

I mean @requires annotation is ignored at build time, so build time validation is still done normally.

Not really. If we ignore the @Requires annotations at build time then we would not be able to detect unsatisfied and ambiguous dependencies properly. Given the example above - if there's a bean which has @Inject Foo then we can't say whether there is a bean that satisfies this injection point. So in theory, we would have to either move all the validation logic to the runtime (no go) or validate some beans during build and those beans affected by any @Requires at runtime.

This approach would make the bootstrap logic more complex, harder to maintain and confusing for users (some errors are reported at build time and some at runtime).

If Quarkus generates DI code like google dagger:

It's actually more complicated than this. We need to support programmatic lookup, bean scopes and so on. Moreover, the CDI spec is very clear that the validation happens before the CDI container is started.

About the native image pre-boot phase, I feel @requires won't be used for too many beans, so pre-boot phase still can optimize many beans, and the user of @requires annotation must be aware of the trade-off.

The real problem is that the CDI container is started when the native image is being built. And not when the native image is executed. And we can't change this because many extensions rely on this behavior.

Dieken commented 3 years ago

I mean @requires annotation is ignored at build time, so build time validation is still done normally.

Not really. If we ignore the @Requires annotations at build time then we would not be able to detect unsatisfied and ambiguous dependencies properly. Given the example above - if there's a bean which has @Inject Foo then we can't say whether there is a bean that satisfies this injection point.

Sorry I don't get your point. When I say "@Requires annotation is ignored at build time", I mean all beans are required at build time, just like there is no @Requires annotation at all, so Quarkus can do fully validation at build time.

I don't mean those @Requires annotated beans are ignored at build time, I mean the @Required annotation itself is ignored at build time.

At runtime, @Requires annotation is evaluated, the fully bean dependency DAG is cut: disabled beans and all beans that recursively depend on the disabled beans are not instantiated. Quarkus doesn't have to do validation at runtime, because those beans depending on disabled beans are not instantiated.

This approach would make the bootstrap logic more complex, harder to maintain and confusing for users (some errors are reported at build time and some at runtime).

The bootstrap logic is indeed more complex, but there won't be bean resolution error at runtime.

If Quarkus generates DI code like google dagger:

It's actually more complicated than this. We need to support programmatic lookup, bean scopes and so on. Moreover, the CDI spec is very clear that the validation happens before the CDI container is started.

As I explained above, the validation is done at build time, not at runtime, although Quarkus ArC will become more complex.

About the native image pre-boot phase, I feel @requires won't be used for too many beans, so pre-boot phase still can optimize many beans, and the user of @requires annotation must be aware of the trade-off.

The real problem is that the CDI container is started when the native image is being built. And not when the native image is executed. And we can't change this because many extensions rely on this behavior.

Thanks for your explanation, let me understand that in a simplified way:

dependency injection happens at build time, this includes: a. calculate the order of bean instantiation b. find argument of class constructor, setter, post construct, and argument for @Inject annotated field c. generate code snippet to organize the logic of the item (a) and (b) just like Google Dagger does
some startup code is executed at build time.

About item 1:

As I explained, the bean resolution still happens fully at build time, item 1.a and 1.b are unchanged, but item 1.c is changed, some runtime conditions must be introduced, just like:

 if (required(objA_id)) {
     objA = new ClassA();
}

This more complex code snippet can still be generated.

About item 2:

According to native-image-pre-boot, it is an optimization for Quarkus framework, it won't be affected if @Requires isn't used by Quarkus framework.

If the pre-boot optimization happens for user code, that means some startup code can't be executed at build time, must be delayed to runtime due to the condition if (required(objA_id)), this will make startup time a bit longer, but I guess it won't affect correction.

mkouba commented 3 years ago

Sorry I don't get your point. When I say "@Requires annotation is ignored at build time", I mean all beans are required at build time, just like there is no @Requires annotation at all, so Quarkus can do fully validation at build time.

That won't work. Imagine you have a bean Foo extends Bar with @Requires(property="my.consumer.enabled", value="true") and a bean Baz extends Bar with @Requires(property="my.consumer.enabled", value="false"). If you ignore @Requires at build time and perform the full validation then @Inject Bar would fail with ambiguous dependency error - there are multiple beans that satisfy this injection point.

According to native-image-pre-boot, it is an optimization for Quarkus framework, it won't be affected if @requires isn't used by Quarkus framework.

The CDI container is used for both framework beans and application beans. The condition if (required(objA_id)) is an oversimplification that does not reflect the real behavior.

Dieken commented 3 years ago

// The last comment is too long, start a new comment.

Back to the original requirement, when I compile a Quarkus app, I don't know which features will be disabled at runtime, I don't want to build dozens of flavors of my Quarkus app: full version, version without scheduler timer, version without consumer for kafka topicA, version without consumer for kafka topicB, etc, the combinations explode.

Luckily we have myMethod.cron.expr=disabled for @Scheduled, but unluckily there is no this way for @Incoming, of course @Incoming can be enhanced to have that ability, but this is not a general solution, I may want to disable some other Quarkus feature at runtime that doesn't provide a switch to disable.

Dieken commented 3 years ago

Sorry I don't get your point. When I say "@Requires annotation is ignored at build time", I mean all beans are required at build time, just like there is no @Requires annotation at all, so Quarkus can do fully validation at build time.

That won't work. Imagine you have a bean Foo extends Bar with @Requires(property="my.consumer.enabled", value="true") and a bean Baz extends Bar with @Requires(property="my.consumer.enabled", value="false"). If you ignore @Requires at build time and perform the full validation then @Inject Bar would fail with ambiguous dependency error - there are multiple beans that satisfy this injection point.

Very valid point👍🏻 I can't figure out a build time solution now except we explicitly forbidden this usage😄

According to native-image-pre-boot, it is an optimization for Quarkus framework, it won't be affected if @requires isn't used by Quarkus framework.

The CDI container is used for both framework beans and application beans. The condition if (required(objA_id)) is an oversimplification that does not reflect the real behavior.

Understand, detail is the devil :-)

quarkusio / quarkus

support conditional bean #19278

Description

Implementation ideas