-
Notifications
You must be signed in to change notification settings - Fork 276
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
TradingStrategy as Spring bean #84
Comments
Hi Filip, I think this is a great idea. I've been slowly chipping away at introducing more Spring over the past months... The bot started off as a pet hobby when I did not know any Spring - hence the homemade dependency injection framework for the strats! I think we need to keep the legacy method of injecting strats - I imagine quite a few folks have existing strats out there now - but we should offer the capability to inject Spring beans in the future like you suggest. I'm planning on getting a release out in the next week to include a Ticker in the API - as suggested by @KrzychuJedi - there'll also be a bunch of dependency upgrades and a general cleanup too. But, for the release after that, including your Spring Bean strat injection change would be ace. Look forward to the PR :-) |
Hi, Eg: I do not want do PR with failing test, but I can not influence these tests. |
PR #85 merged. |
* Also made class-name vs bean-name mutually exclusive in XSD * Updated sample strat config * Added some extra unit tests * Couple of other minor javadoc updates
Released. |
I would found useful if it were possible to instantiate TradingStrategie implementations by Spring (eg. I would like to autowire Spring manage JPA repository into strategy to save audit info into database during strategy execution).
This could be done by adding new config parameter to strategies.xml (something like bean-name) and during TradingEngine#loadMarketConfigAndInitialiseTradingStrategies() phase looks up the bean at SpringContext.
This behavior could be complementary to TradingStrategy instantiation by class-name.
Please let me know what do you think of it. I have this done at my local fork and after some enhancement I could push it to your branch, if you want to.
The text was updated successfully, but these errors were encountered: