You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/01/13 15:56:51 UTC
[GitHub] [camel-quarkus] ppalaga opened a new issue #617: Build time Camel
service resolution
ppalaga opened a new issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617
I stumbled upon this when I was rebasing my telegram branch on top of the current Camel master. It did not work because the endpoint and component configurers were not found. Those are currently looked up at runtime by inspecting the files in `META-INF/services/org/apache/camel/configurer` and we do not add them to the native image.
Clearly, the problem could be solved by adding `META-INF/services/org/apache/camel/configurer` & co. to the native image, but that kind of solution would not utilize the full potential of Quarkus build time augmentation.
I am currently having a PoC that provides a camel-quarkus specific implementation of FactoryFinderResolver. It resolves the services based on a single map that is filled at build time.
I wonder whether anybody can think of a better strategy here?
I'll share my code when I get all tests passing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #617: Build time
Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-573754643
as far as I can remember the resolution always checks the registry and after the factory finder, this is how component, languages and other bits are actually resolved
if does not it seems an error but I remember it should be already the case for the configurer
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574650783
> After brainstorming a little bit more with @davsclaus we probably need to revisit how we load such services and we probably need a smart FactoryFinders which takes over the role of the registry for most of the discovery so we can keep the usual camel behaviour where registry is used to provide alternative implementations for what the it is defined by service factories.
Sounds good.
> We probably can have some build items that control how the discovered services are handled i.e.:
>
> 1. a build item can control that some services need to be instantiated and they should probably a good candidates to be stored into the registry (i.e. components)
Yes, that's what we do for all (except filter-blacklisted) services ATM.
> 2. a build item can control what services need to be discovered
Could you please define "need to be discovered"? Is it somehow to imply, that we are going to make some services non-discoverable at runtime (e.g. by not including the service file in the app)?
> 3. a build item can control what service files have to be backed into the final native image
> 4. a build item can control what service files have to be "pre-loaded" in the form of a class
What possible reasons are there for having both 3. and 4.? They seem to be serving the same purpose, don't they? Having just one of them would make our code simpler.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #617: Build time
Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-575169436
will open a new issue with a proposal so we can have a better discussion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] davsclaus commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
davsclaus commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-573768756
the configurers are source code generated and loaded from classpath and as such these are not looked up anywhere. So its because you are doing something "different" in quarkus.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli edited a comment on issue #617: Build
time Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli edited a comment on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574186630
@ppalaga
After brainstorming a little bit more with @davsclaus we probably need to revisit how we load such services and we probably need a smart FactoryFinders which takes over the role of the registry for most of the discovery so we can keep the usual camel behaviour where registry is used to provide alternative implementations for what the it is defined by service factories.
We probably can have some build items that control how the discovered services are handled i.e.:
1. a build item can control that some services need to be instantiated and they should probably a good candidates to be stored into the registry (i.e. components)
2. a build item can control what services need to be discovered
3. a build item can control what service files have to be backed into the final native image
4. a build item can control what service files have to be "pre-loaded" in the form of a class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-573750337
> I think all those services are already available from the registry
I also suspected this might be true but I was not able to conceive any possibility how this fact could help. Maybe you mean that [DefaultComponent.doInit()](https://github.com/apache/camel/blob/master/core/camel-support/src/main/java/org/apache/camel/support/DefaultComponent.java#L408-L422) should prefer getting the configurers from the registry instead of from FactoryFinder?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli edited a comment on issue #617: Build
time Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli edited a comment on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574061478
Yep but the factory finder does not actually solve the issue as the factory instance need to be in any case instantiated and put into another registry, otherwise we still need to look it up, do class for name, new instance at runtime.
Another option would be to actually discover such configurers at build time and register them through a recorder but the object won't be deleted as the component hold a reference so there's no real difference (IMHO) and this holds true for any object that is bound to a component.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] davsclaus commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
davsclaus commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574059375
Isn't a problem with adding everything to the registry is that you clutter up and takes spaces there. Also for stuff that was only needed during build/startup time. Those will reside in Registry and wont be eligble to be removed.
For the endpoint and component configurers then yeah they may be used during runtime if you create new endpoints / components.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #617: Build time
Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-573735391
I think all those services are already available from the registry
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #617: Build time
Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574084930
> In general, I do not mind prefering the Camel registry. It is there and can serve the purpose of instantiating services well.
>
> The problem I am trying to solve here is, that we (camel-quarkus) still have code assuming that the FactoryFinders work well (incl. native), which they currently don't. I see several options how to fix that:
>
> A. Brute force: always add all service property files to the native image and keep using `DefaultFactoryFinderResolver` / `DefaultFactoryFinder` which read the files at runtime.
>
> B. Totally avoid using FactoryFinders. Provide an exception throwing FactoryFinder to make sure that our extensions do not use it at all. I am not sure this is possible and I am not sure this is a good idea, because third party extensions may still assume the FactoryFinders work.
>
> C. My original proposal: Custom FactoryFinder assembled at build time, no need to include the property files in the native image.
The problem I see here is: what services do you add to the new factory finder "map" and what do you add to the registry ?
I guess at the end we do end up having the same service registered to different "registries", because we do not know what is the patter 3th party libraries are using, isn't it ?
May be we should have the factory finder to hooks into the registry or move every discovered service from the camel registry to the factory finder (but this requires some other changes in the FastCamelContext)
> Both A. and C. sound like acceptable solutions to me, C. being more in accordance with the spirit of Quarkus.
An intermediate solution would be to have an option to
1. store the service files in the native image
2. have the factory finder to emit a warning when it loads from a file
> WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli edited a comment on issue #617: Build
time Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli edited a comment on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574061478
Yep but the factory finder does not actually solve the issue as the factory instance need to be in any case instantiated and put into another registry, otherwise we still need to look it up, do class for name, new instance at runtime.
Another option would be to actually discover such configurers at build time and register them through a recorder but the object won't be deleted as the component hold a reference so there's no real difference (IMHO) and this holds true for any object that is bound to the a component.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #617: Build time
Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574061478
Yep but the factory finder does not actually solve the issue as the factory instance need to be in any case instantiated and put into another registry, otherwise we still need to look it up, do class for name, new instance at runtime.
Another option would be to actually discover such configurers at build time and register them through a recorder.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574054112
I have sent a PR to Camel https://github.com/apache/camel/pull/3478 It makes DefaultComponent.doInit() to prefer getting the configurers from the registry before checking the FactoryFinder and it solves the original problem with missing configurers in the native mode.
Regardless of that, I think we should have a camel-quarkus specific implementation of FactoryFinderResolver that would be assembled at build time, so that the service property files do not need to get parsed at runtime. Let me share my code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-573755212
No, it checks only the FactoryFinder https://github.com/apache/camel/blob/master/core/camel-support/src/main/java/org/apache/camel/support/DefaultComponent.java#L408-L422
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-575251057
I have updated https://github.com/apache/camel-quarkus/pull/618 Now it contains a minimalist implementation of 1., 2. and 4.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-573765094
Filed https://issues.apache.org/jira/browse/CAMEL-14397
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #617: Build time
Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574186630
@ppalaga
After brainstorming a little bit more with @davsclaus we probably need to revisit how we load such services and we probably need a smart FactoryFinders which takes over the role of the registry for most of the discovery so we can keep the usual camel behaviour where registry is used to provide alternative implementations for what the it is defined by service factories.
We probably can have some build items that control how the discovered services are handled i.e.:
1. a build item can control that some services need to be instantiated and they should probably a good candidates to be stored into the registry
2. a build item can control what services need to be discovered
3. a build item can control what service files have to be backed into the final native image
4. a build item can control what service files have to be "pre-loaded" in the form of a class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574077814
In general, I do not mind prefering the Camel registry. It is there and can serve the purpose of instantiating services well.
The problem I am trying to solve here is, that we (camel-quarkus) still have code assuming that the FactoryFinders work well (incl. native), which they currently don't. I see several options how to fix that:
A. Brute force: always add all service property files to the native image and keep using `DefaultFactoryFinderResolver` / `DefaultFactoryFinder` which read the files at runtime.
B. Totally avoid using FactoryFinders. Provide an exception throwing FactoryFinder to make sure that our extensions do not use it at all. I am not sure this is possible and I am not sure this is a good idea, because third party extensions may still assume the FactoryFinders work.
C. My original proposal: Custom FactoryFinder assembled at build time, no need to include the property files in the native image.
Both A. and C. sound like acceptable solutions to me, C. being more in accordance with the spirit of Quarkus.
WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli edited a comment on issue #617: Build
time Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli edited a comment on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574057397
I'm not 100% sure about the need of a specific factory finder as we do already have the registry, we probably need to follow the same path when it is about tor resolve services so we don't need to have platform specific code.
As example the resolver for components, languages and dataformats do not need the factory finder at all so if we introduce a factory finder we need to use it everywhere otherwise it may result inconsistent.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #617: Build time
Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-573758834
mh strange, I think that code should be fixed unless @davsclaus did it on purpose.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #617: Build time
Camel service resolution
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574057397
I'm not 100% sure about the need of a specific factory finder as we do already have the registry, we probably need to follow the same path when it is about tor resolve services so we don't need to have platform specific code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-575167277
> 3. a build item can control what service files have to be backed into the final native image
> 4. a build item can control what service files have to be "pre-loaded" in the form of a class
If we had just 4. that would actually mean that the FactoryFinders work the same way for both JVM and native, so it would be easier to spot issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli closed issue #617: Registerable and
discoverable Camel services
Posted by GitBox <gi...@apache.org>.
lburgazzoli closed issue #617: Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/issues/617
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #617: Build time Camel
service resolution
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #617: Build time Camel service resolution
URL: https://github.com/apache/camel-quarkus/issues/617#issuecomment-574115196
> > C. My original proposal: Custom FactoryFinder assembled at build time, no need to include the property files in the native image.
>
> The problem I see here is: what services do you add to the new factory finder "map"
The map is from property file paths (e.g. `META-INF/services/org/apache/camel/configurer/telegram-component`) to classes (e.g. `org.apache.camel.component.telegram.TelegramComponentConfigurer.class`) and my current implementation adds all available paths and classes to the map. Here is the code: https://github.com/apache/camel-quarkus/pull/618
> and what do you add to the registry ?
No change against what we currently do.
My understanding is that FactoryFinders and Registry serve different, but slightly overlapping purposes. Most notably, a FactoryFinder can find a class by path + name, which Registry cannot.
> I guess at the end we do end up having the same service registered to different "registries", because we do not know what is the patter 3th party libraries are using, isn't it ?
Yes, FactoryFinders and the registry will contain slightly overlapping data, but not 100% identical.
The access keys are also different: FactoryFinders use hierarchic names (they include the resource path), while the registry names are flat.
> May be we should have the factory finder to hooks into the registry or move every discovered service from the camel registry to the factory finder (but this requires some other changes in the FastCamelContext)
I am not sure FactoryFinders backed by a Registry covering all cases would be possible. Sounds like you assume the property file name is always equal to the name of the service in the registry. Based on @davsclaus comments in https://github.com/apache/camel/pull/3478#discussion_r366198855 I am not sure this is always true.
Besides, is your FactoryFinders backed by a Registry meant to happen in Camel or here?
> > Both A. and C. sound like acceptable solutions to me, C. being more in accordance with the spirit of Quarkus.
>
> An intermediate solution would be to have an option to
>
> 1. store the service files in the native image
> 2. have the factory finder to emit a warning when it loads from a file
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services