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

Enable auto-discovery of camel routes defined in a configured package. Added a Demo #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

girishvasmatkar
Copy link

  • Enable auto-discovery of camel routes defined in a configured package.
  • Added a Demo Route that reads JSON file from file system and calls a pre-defined Moqui service.

@girishvasmatkar
Copy link
Author

@jonesde : Please review and see if it is worth adding to the moqui-camel source code.

@jonesde
Copy link
Member

jonesde commented Aug 25, 2019

Routes can be added using the CamelContext from the existing tool factory, either with a custom ToolFactory or with startup actions in the Moqui Conf XML file (ie MoquiConf.xml in your add-on component).

If we were to include something like this is would be redundant with what Camel already supports but if we did then we'd probably want something more flexible than a single property or env var for a package scan. BTW, from a quick read through of the code it looks like you only do the package scan if the system property is set, but it appears to ignore the value of the system property and use a constant instead (and it's a org.moqui package which would be a bad practice for custom add-ons to Moqui).

My first thought is that this isn't a significant value add and more importantly I don't have enough experience to properly design such a thing based on real world needs where it might be applicable.

@girishvasmatkar
Copy link
Author

Thanks for the detailed review, David. It gives good insight about what better can be done for adding routes to the context and start up actions (which was unknown for me so far), can be used for the same.
Regarding, ignoring the system property, this was probably a mistake to let go of the property to get the package name.
Thanks for your valuable time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants