Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CAMEL-14215: Adding camel-main-osgi. #3378

Closed

Conversation

@bobpaulin
Copy link
Contributor

bobpaulin commented Dec 3, 2019

https://issues.apache.org/jira/browse/CAMEL-14215

@jbonofre @oscerd - Resubmitted PR cleaned up for 3.1.0. Sorry for the noise.

@oscerd
oscerd approved these changes Dec 3, 2019
@oscerd

This comment has been minimized.

Copy link
Contributor

oscerd commented Dec 3, 2019

Thanks, this has been merged on master.

@oscerd oscerd closed this Dec 3, 2019
@davsclaus

This comment has been minimized.

Copy link
Contributor

davsclaus commented Dec 3, 2019

Oh I planned to do a little review. So on top of my head

  • this is NOT related to camel-main (eg dont use/extend camel-main) so the name is misleading/bad
  • you start camel context before adding routes, that is not how camel normally do, so you end up with camel saying --- Total 0 routes started.
  • there is no documentation in src/main/docs
  • also it seems you dont do like camel-core-osgi where it does some osgihelper to init something with osgi to setup various bits (i think you need to do this manually too, unless its somehow still done)

Can we get this corrected. And also give it a new name for example camel-core-osgi-activator

And then maybe add an example in the examples that uses this new stuff

@bobpaulin

This comment has been minimized.

Copy link
Contributor Author

bobpaulin commented Dec 3, 2019

Hi @davsclaus , Thank you for the review. Comments inline

this is NOT related to camel-main (eg dont use/extend camel-main) so the name is misleading/bad

Agree it is not using/extending camel--main. Naming is hard I was looking for something close conceptually. I like the suggestion camel-core-osgi-activator. Will update this.

you start camel context before adding routes, that is not how camel normally do, so you end up with camel saying --- Total 0 routes started.

Yes there are 2 different ways routes are added. At startup I check for existing RouteBuilder services registered and then afterward I use the service tracker to notify the context of new RouteBuilder services added later. I thought it would be more consistent to add all routes after starting. You mention that is not how it is normally done, is there a reason for this? Is my approach harmful? The idea of this is to allow camel to take advantage of OSGi's dynamic nature.

there is no documentation in src/main/docs
I can start with something like this https://github.com/bobpaulin/camel-main-osgi/blob/master/README.md

also it seems you dont do like camel-core-osgi where it does some osgihelper to init something with osgi to setup various bits (i think you need to do this manually too, unless its somehow still done)

I might need more specific direction here. I do see camel-core-osgi takes a different approach to registering the osgi services. But I'm not sure if that's what you mean. Can you provide more details?

Thanks again for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.