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

TradingStrategy as Spring bean #84

Closed
kuceraf opened this issue Dec 16, 2017 · 5 comments
Closed

TradingStrategy as Spring bean #84

kuceraf opened this issue Dec 16, 2017 · 5 comments
Assignees
Projects

Comments

@kuceraf
Copy link
Contributor

kuceraf commented Dec 16, 2017

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.

kuceraf added a commit to kuceraf/bxbot that referenced this issue Dec 16, 2017
kuceraf added a commit to kuceraf/bxbot that referenced this issue Dec 16, 2017
@gazbert
Copy link
Owner

gazbert commented Dec 18, 2017

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 :-)

kuceraf added a commit to kuceraf/bxbot that referenced this issue Dec 23, 2017
kuceraf added a commit to kuceraf/bxbot that referenced this issue Dec 23, 2017
@kuceraf
Copy link
Contributor Author

kuceraf commented Dec 23, 2017

Hi,
I've pushed my commits related to this task to remote branch ready for PR. But some test are failing on Travis CI.

Eg:
GeminiIT#testPublicApiCalls() test sends some HTTP request to https://api.gemini.com/v1/pubticker/ethbtc and it fails on 503. I think this test is dependend on the remote exchange state.

I do not want do PR with failing test, but I can not influence these tests.
Any idea how to solve it?

kuceraf added a commit to kuceraf/bxbot that referenced this issue Dec 24, 2017
@gazbert gazbert self-assigned this Dec 24, 2017
@gazbert
Copy link
Owner

gazbert commented Dec 24, 2017

#85 - latest PR to be reviewed by @gazbert

@gazbert gazbert closed this as completed in 4802247 Jan 2, 2018
@gazbert
Copy link
Owner

gazbert commented Jan 2, 2018

PR #85 merged.
Re-opened to add doc updates this week...

@gazbert gazbert reopened this Jan 2, 2018
@gazbert gazbert added this to In Progress in BX-bot Jan 2, 2018
gazbert added a commit that referenced this issue Jan 8, 2018
* 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
@gazbert
Copy link
Owner

gazbert commented Feb 4, 2018

Released.

@gazbert gazbert closed this as completed Feb 4, 2018
@gazbert gazbert moved this from In Progress to Done in BX-bot Feb 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

2 participants