Skip to content

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

Open
girishvasmatkar wants to merge 2 commits into
moqui:masterfrom
girishvasmatkar:master
Open

Enable auto-discovery of camel routes defined in a configured package. Added a Demo#1
girishvasmatkar wants to merge 2 commits into
moqui:masterfrom
girishvasmatkar:master

Conversation

@girishvasmatkar
Copy link
Copy Markdown

  • 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
Copy Markdown
Author

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

@jonesde
Copy link
Copy Markdown
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
Copy Markdown
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