gazbert / bxbot

A simple Bitcoin trading bot written in Java.
MIT License
779 stars 275 forks source link

TradingStrategy as Spring bean #84

Closed kuceraf closed 6 years ago

kuceraf commented 6 years ago

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.

gazbert commented 6 years ago

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 commented 6 years ago

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?

gazbert commented 6 years ago

85 - latest PR to be reviewed by @gazbert

gazbert commented 6 years ago

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

gazbert commented 6 years ago

Released.