Skip to content

Conversation

middagj
Copy link
Contributor

@middagj middagj commented Dec 4, 2019

If no scope is used on a Produces method, it defaults to Dependent, which is probably not the best for the ConnectionFactory and ServerLocator.

@gastaldi
Copy link
Contributor

gastaldi commented Dec 4, 2019

Maybe add a @DefaultBean too so users can override it if necessary?

@middagj
Copy link
Contributor Author

middagj commented Dec 4, 2019

Yes, that is a good addition. I have added that.

Also we have in our custom extension to set up some JMS a problem with the ordering so I added a ArtemisConfiguredBuildItem that can be used for ordering. It is a common pattern in other extensions, only there it is done via a SimpleBuildItem with a RuntimeValue of the produced item. Using a SimpleBuildItem is not feasible because we have two methods producing the same item, which is not allowed. I only plan to use it for ordering and get the bean from the BeanContainer.

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 4, 2019
@gsmet
Copy link
Member

gsmet commented Dec 4, 2019

Looks good, thanks.

@gsmet
Copy link
Member

gsmet commented Dec 4, 2019

@middagj btw, have you tried your @Produce trick? It's the first time I saw this one in action :).

@middagj
Copy link
Contributor Author

middagj commented Dec 4, 2019

No, but I can check of course.

@gsmet
Copy link
Member

gsmet commented Dec 4, 2019

Yes, better check it works OK.

@geoand
Copy link
Contributor

geoand commented Dec 5, 2019

So is this good to go?

@gsmet
Copy link
Member

gsmet commented Dec 5, 2019

@geoand not really, I'm waiting for @middagj to test his @Produce thing works as he expects.

@geoand geoand removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 5, 2019
@geoand
Copy link
Contributor

geoand commented Dec 5, 2019

OK thanks! I removed the waiting-for-ci label then to avoid having this merged by accident

@middagj
Copy link
Contributor Author

middagj commented Dec 5, 2019

Sorry, it took me sometime to force a wrong initial order as it comes always ordered on my mac (only seems to happen on Linux in the CI/CD pipeline).

It does not work when producing the jar (it does work in testing), I get a:

Failed to build a runnable JAR: Failed to augment application classes: Cycle detected:
[ERROR]            io.quarkus.artemis.jms.deployment.ArtemisJmsProcessor#configure produced class io.quarkus.artemis.core.deployment.ArtemisConfiguredBuildItem
[ERROR]         to org.acme.CustomProcessor#defineBeanAnnotation produced class io.quarkus.arc.deployment.BeanDefiningAnnotationBuildItem
[ERROR]         to io.quarkus.arc.deployment.BeanArchiveProcessor#build produced class io.quarkus.arc.deployment.BeanArchiveIndexBuildItem
[ERROR]         to io.quarkus.arc.deployment.AutoInjectFieldProcessor#autoInjectQualifiers produced class io.quarkus.arc.deployment.AutoInjectAnnotationBuildItem
[ERROR]         to io.quarkus.arc.deployment.AutoInjectFieldProcessor#annotationTransformer produced class io.quarkus.arc.deployment.AnnotationsTransformerBuildItem
[ERROR]         to io.quarkus.arc.deployment.ArcProcessor#initialize produced class io.quarkus.arc.deployment.ContextRegistrationPhaseBuildItem
[ERROR]         to io.quarkus.arc.deployment.ArcProcessor#registerBeans produced class io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem
[ERROR]         to io.quarkus.arc.deployment.ConfigBuildStep#analyzeConfigPropertyInjectionPoints produced class io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem$BeanConfiguratorBuildItem
[ERROR]         to io.quarkus.arc.deployment.ArcProcessor#validate produced class io.quarkus.arc.deployment.ValidationPhaseBuildItem
[ERROR]         to io.quarkus.arc.deployment.ArcProcessor#generateResources produced class io.quarkus.arc.deployment.BeanContainerBuildItem
[ERROR]         to io.quarkus.artemis.jms.deployment.ArtemisJmsProcessor#configure

I changed the implementation to use separate SimpleBuildItems.

@gsmet
Copy link
Member

gsmet commented Dec 6, 2019

Thanks for checking.

@gsmet gsmet merged commit 152ddb7 into quarkusio:master Dec 6, 2019
@gsmet gsmet added this to the 1.1.0 milestone Dec 6, 2019
@middagj middagj deleted the feature/artemis-producer branch December 6, 2019 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants