ArC - Warning about CDI Lite and bean discovery mode

[gsmet]

I just started one of my applications and I ended up with the following warning at startup:

2023-02-23 18:17:47,306 WARN  [io.qua.arc.dep.BeanArchiveProcessor] (build-24) Detected bean archive with bean discovery mode of 'all', this is not portable in CDI Lite and is treated as 'annotated' in Quarkus! Path to beans.xml: /META-INF/beans.xml

The first step in improving the situation is to improve the warn message to include the artifact as with the current message, it's impossible to know which dependency is problematic (I don't have a beans.xml in my app, this is coming from a dependency).
I already have a patch for it and will open a PR.

While I understand the rationale of it, we need to make sure that all our extensions are free of this or our users will all start seeing these warnings.

For instance, SmallRye GraphQL is problematic: https://github.com/smallrye/smallrye-graphql/blob/main/client/implementation/src/main/resources/META-INF/beans.xml .

And there might be others.

So we would need to go through the full CI log and check the occurrences of this message once the improved error message is merged.

[quarkus-bot]

/cc @Ladicek (arc), @manovotn (arc), @mkouba (arc)

[gsmet]

First step is here: #31379

[manovotn]

While I understand the rationale of it, we need to make sure that all our extensions are free of this or our users will all start seeing these warnings.

Good point, I didn't think we have that since up until now we just detected beans.xml as a marker and disregarded its content; silly me I guess.
Technically, we don't need to have this warning at all - I just thought I'd be better to tell users since they might reuse setup from other projects and rely on discovery mode that's not supported.
However, there can also be a case where you use a 3rd party lib that has this setup and you cannot do anything about it; then this logging can be annoying. We could have a knob for it, but that sounds as an overkill for just logging.

So we would need to go through the full CI log and check the occurrences of this message once the improved error message is merged.

Or change the warning to throwing an exception on a separate PR to let CI crash on the problematic bits :-)

[gsmet]

OK, the outcome is (better use a log than an exception to have everything in one go):

     54 io.smallrye:smallrye-graphql-cdi:/META-INF/beans.xml
     13 io.smallrye:smallrye-graphql-client:/META-INF/beans.xml
     54 io.smallrye:smallrye-graphql:/META-INF/beans.xml
     14 org.jboss.narayana.rts:lra-client:/META-INF/beans.xml

Now my question: can we safely change the value in these components? Because they are all used outside of the context of Quarkus so I'm not sure if it's something we can/should do.

If we can't fix it or we expect other libraries to come with the same thing, I would recommend to make this message debug instead of warn.

[manovotn]

Now my question: can we safely change the value in these components? Because they are all used outside of the context of Quarkus so I'm not sure if it's something we can/should do.

For the sake of Quarkus, you can safely change them to annotated. For usages outside of Quarkus, you cannot know how or what they rely on.

I am sure you know this but just for the sake of reader's clarity, the difference between annotated and all is that all processes all classes (even those with no annotations) and turns all eligible ones into beans. So the possible culprit is that someone, somewhere can rely on an unannotated class inside smallrye-graphql to be a bean they can @Inject.

It's mostly fairly easy to transition from all to annotated - simply make sure that all the classes that are expected to become beans have some bean defining annotation (mostly a scope). This can even be a no-op in many cases and graphql might be one of them as we successfully integrate it within Quarkus even where it "expects" all discovery mode.

If we can't fix it or we expect other libraries to come with the same thing, I would recommend to make this message debug instead of warn.

We should IMO fix our libraries and I don't expect that to be too difficult.
For 3rd party libraries I believe this warning is beneficial as it indicates the library may not function properly in Quarkus (or CDI Lite in general)

[gsmet]

We should IMO fix our libraries and I don't expect that to be too difficult.

Could you reach out to the authors of these libraries and provide recommendations?

That would be @mmusgrov for Narayana LRA and @jmartisk for SmallRye GraphQL. The latter being the priority as I think it's more largely used.

Thanks.

[manovotn]

Of course, I'll first take a look at those JARs myself to see what's in there and then continue that discussion via email :)

[mmusgrov]

The LRA client module only needs bean-discovery-mode "annotated" so I've raised https://issues.redhat.com/browse/JBTM-3756 and a pr to fix it.

[manovotn]

@mmusgrov oh, you beat me to it by few minutes :)
I'll leave some comments on that PR, thanks!

[manovotn]

@gsmet FYI this is now fixed in SR graphql and in narayana. Therefore, future releases of both libraries should make the warning go away. I assume we want to keep this issue open until we upgrade graphql and narayana versions.

[melloware]

I get this warning now in my OmniFaces wrapper extension. OmniFaces configured its beans.xml this way intentionally as it's a utility library.

Should I ask the author whether it should switch to annotated?

[manovotn]

I get this warning now in my OmniFaces wrapper extension. OmniFaces configured its beans.xml this way intentionally as it's a utility library.

Should I ask the author whether it should switch to annotated?

If the library wishes to support CDI Lite, then annotated is required.
Quarkus will treat all mode as annonated nonetheless and did so up until now so the lib should work the same for you if you just changed the discovery mode.
If there are any changes needed WRT EE usage, those should be minimal.

[gsmet]

@manovotn I wonder if we should have a build item so that extension authors can avoid the warnings for libraries that are known to be working.
I'm less than sure that we will have the traction to make all the upstream projects change their config.

[manovotn]

True I thought about it as well. But I am on PTO now so I will defer to @Ladicek and @mkouba :⁠-⁠)

[Ladicek]

A build item that would carry the artifact coordinates makes sense to me (at least a lot more sense than a config flag for disabling the warning for good :-) ). I'll try to stitch something together.

[Ladicek]

Here's a draft PR: #31856

@melloware could you please take a look? I believe it is self-explanatory and should cover your case well.

[melloware]

That looks good to me and would handle my needs perfectly.

[gsmet]

I think we can close this one: we have a build item and SmallRye GraphQL has been upgraded in main and it was my main concern.
Thanks everyone.

[Ladicek]

We don't have a build item yet, because my PR is still draft :-) I'll rebase and undraft in a bit though, so we should be good soon.

[melloware]

I tried the build item:

    @BuildStep
    void produceKnownCompatible(BuildProducer<KnownCompatibleBeanArchiveBuildItem> knownCompatibleProducer) {
        // GitHub #62: bean discovery mode in beans.xml
        knownCompatibleProducer.produce(new KnownCompatibleBeanArchiveBuildItem("org.omnifaces", "omnifaces"));
    }

But it does not work I still see this warning:

2023-03-25 07:39:06,833 WARN  [io.qua.arc.dep.BeanArchiveProcessor] (build-31) 
Detected bean archive with bean discovery mode of 'all', this is not portable in CDI Lite and is treated as 'annotated' in Quarkus! Path to beans.xml: org.omnifaces:omnifaces:/META-INF/beans.xml