From 74db82d759f2651e916c6e6ae25250ae637db7be Mon Sep 17 00:00:00 2001 From: gcarq Date: Sat, 9 Jun 2018 01:19:42 +0200 Subject: [PATCH 1/2] main: don't touch freqbot state in cleanup() cleanup() should be only called after the main loop has been exited. At that point the state shouldn't be modified. --- freqtrade/freqtradebot.py | 9 +++------ freqtrade/main.py | 3 ++- freqtrade/tests/test_freqtradebot.py | 11 ++++------- freqtrade/tests/test_main.py | 6 +++--- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index cd0c4b6d4..e1c1c9e65 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -76,17 +76,14 @@ class FreqtradeBot(object): else: self.state = State.STOPPED - def clean(self) -> bool: + def cleanup(self) -> None: """ - Cleanup the application state und finish all pending tasks + Cleanup pending resources on an already stopped bot :return: None """ - self.rpc.send_msg('*Status:* `Stopping trader...`') - logger.info('Stopping trader and cleaning up modules...') - self.state = State.STOPPED + logger.info('Cleaning up modules ...') self.rpc.cleanup() persistence.cleanup() - return True def worker(self, old_state: State = None) -> State: """ diff --git a/freqtrade/main.py b/freqtrade/main.py index 81e578810..903b779d3 100755 --- a/freqtrade/main.py +++ b/freqtrade/main.py @@ -55,7 +55,8 @@ def main(sysargv: List[str]) -> None: logger.exception('Fatal exception!') finally: if freqtrade: - freqtrade.clean() + freqtrade.rpc.send_msg('*Status:* `Stopping trader...`') + freqtrade.cleanup() sys.exit(return_code) diff --git a/freqtrade/tests/test_freqtradebot.py b/freqtrade/tests/test_freqtradebot.py index aca6dce2f..5339ebc24 100644 --- a/freqtrade/tests/test_freqtradebot.py +++ b/freqtrade/tests/test_freqtradebot.py @@ -68,7 +68,7 @@ def test_freqtradebot_object() -> None: Test the FreqtradeBot object has the mandatory public methods """ assert hasattr(FreqtradeBot, 'worker') - assert hasattr(FreqtradeBot, 'clean') + assert hasattr(FreqtradeBot, 'cleanup') assert hasattr(FreqtradeBot, 'create_trade') assert hasattr(FreqtradeBot, 'get_target_bid') assert hasattr(FreqtradeBot, 'process_maybe_execute_buy') @@ -93,7 +93,7 @@ def test_freqtradebot(mocker, default_conf) -> None: assert freqtrade.state is State.STOPPED -def test_clean(mocker, default_conf, caplog) -> None: +def test_cleanup(mocker, default_conf, caplog) -> None: """ Test clean() method """ @@ -101,11 +101,8 @@ def test_clean(mocker, default_conf, caplog) -> None: mocker.patch('freqtrade.persistence.cleanup', mock_cleanup) freqtrade = get_patched_freqtradebot(mocker, default_conf) - assert freqtrade.state == State.RUNNING - - assert freqtrade.clean() - assert freqtrade.state == State.STOPPED - assert log_has('Stopping trader and cleaning up modules...', caplog.record_tuples) + freqtrade.cleanup() + assert log_has('Cleaning up modules ...', caplog.record_tuples) assert mock_cleanup.call_count == 1 diff --git a/freqtrade/tests/test_main.py b/freqtrade/tests/test_main.py index 942bd8687..82d8a8b34 100644 --- a/freqtrade/tests/test_main.py +++ b/freqtrade/tests/test_main.py @@ -70,7 +70,7 @@ def test_main_fatal_exception(mocker, default_conf, caplog) -> None: 'freqtrade.freqtradebot.FreqtradeBot', _init_modules=MagicMock(), worker=MagicMock(side_effect=Exception), - clean=MagicMock(), + cleanup=MagicMock(), ) mocker.patch( 'freqtrade.configuration.Configuration._load_config_file', @@ -97,7 +97,7 @@ def test_main_keyboard_interrupt(mocker, default_conf, caplog) -> None: 'freqtrade.freqtradebot.FreqtradeBot', _init_modules=MagicMock(), worker=MagicMock(side_effect=KeyboardInterrupt), - clean=MagicMock(), + cleanup=MagicMock(), ) mocker.patch( 'freqtrade.configuration.Configuration._load_config_file', @@ -124,7 +124,7 @@ def test_main_operational_exception(mocker, default_conf, caplog) -> None: 'freqtrade.freqtradebot.FreqtradeBot', _init_modules=MagicMock(), worker=MagicMock(side_effect=OperationalException('Oh snap!')), - clean=MagicMock(), + cleanup=MagicMock(), ) mocker.patch( 'freqtrade.configuration.Configuration._load_config_file', From 0b5d21f32af7381f3fdbd317c68a1280523a11da Mon Sep 17 00:00:00 2001 From: gcarq Date: Sat, 9 Jun 2018 04:29:48 +0200 Subject: [PATCH 2/2] implement bot reconfiguration and expose it to telegram --- docs/telegram-usage.md | 1 + freqtrade/main.py | 23 +++++++- freqtrade/rpc/rpc.py | 5 ++ freqtrade/rpc/telegram.py | 13 +++++ freqtrade/state.py | 3 +- freqtrade/tests/rpc/test_rpc_telegram.py | 27 ++++++++- freqtrade/tests/test_main.py | 72 +++++++++++++++++++++++- 7 files changed, 139 insertions(+), 5 deletions(-) diff --git a/docs/telegram-usage.md b/docs/telegram-usage.md index 4f878dbd1..ca764c9b4 100644 --- a/docs/telegram-usage.md +++ b/docs/telegram-usage.md @@ -16,6 +16,7 @@ official commands. You can ask at any moment for help with `/help`. |----------|---------|-------------| | `/start` | | Starts the trader | `/stop` | | Stops the trader +| `/reload_conf` | | Reloads the configuration file | `/status` | | Lists all open trades | `/status table` | | List all open trades in a table format | `/count` | | Displays number of trades used and available diff --git a/freqtrade/main.py b/freqtrade/main.py index 903b779d3..9d17a403a 100755 --- a/freqtrade/main.py +++ b/freqtrade/main.py @@ -5,12 +5,14 @@ Read the documentation to know what cli arguments you need. """ import logging import sys +from argparse import Namespace from typing import List from freqtrade import OperationalException from freqtrade.arguments import Arguments from freqtrade.configuration import Configuration from freqtrade.freqtradebot import FreqtradeBot +from freqtrade.state import State logger = logging.getLogger('freqtrade') @@ -44,6 +46,8 @@ def main(sysargv: List[str]) -> None: state = None while 1: state = freqtrade.worker(old_state=state) + if state == State.RELOAD_CONF: + freqtrade = reconfigure(freqtrade, args) except KeyboardInterrupt: logger.info('SIGINT received, aborting ...') @@ -55,11 +59,28 @@ def main(sysargv: List[str]) -> None: logger.exception('Fatal exception!') finally: if freqtrade: - freqtrade.rpc.send_msg('*Status:* `Stopping trader...`') + freqtrade.rpc.send_msg('*Status:* `Process died ...`') freqtrade.cleanup() sys.exit(return_code) +def reconfigure(freqtrade: FreqtradeBot, args: Namespace) -> FreqtradeBot: + """ + Cleans up current instance, reloads the configuration and returns the new instance + """ + # Clean up current modules + freqtrade.cleanup() + + # Create new instance + freqtrade = FreqtradeBot(Configuration(args).get_config()) + freqtrade.rpc.send_msg( + '*Status:* `Config reloaded ...`'.format( + freqtrade.state.name.lower() + ) + ) + return freqtrade + + def set_loggers() -> None: """ Set the logger level for Third party libs diff --git a/freqtrade/rpc/rpc.py b/freqtrade/rpc/rpc.py index c2f097319..f1eecbc18 100644 --- a/freqtrade/rpc/rpc.py +++ b/freqtrade/rpc/rpc.py @@ -299,6 +299,11 @@ class RPC(object): return True, '*Status:* `already stopped`' + def rpc_reload_conf(self) -> str: + """ Handler for reload_conf. """ + self.freqtrade.state = State.RELOAD_CONF + return '*Status:* `Reloading config ...`' + # FIX: no test for this!!!! def rpc_forcesell(self, trade_id) -> Tuple[bool, Any]: """ diff --git a/freqtrade/rpc/telegram.py b/freqtrade/rpc/telegram.py index c110b9627..43383fe43 100644 --- a/freqtrade/rpc/telegram.py +++ b/freqtrade/rpc/telegram.py @@ -93,6 +93,7 @@ class Telegram(RPC): CommandHandler('performance', self._performance), CommandHandler('daily', self._daily), CommandHandler('count', self._count), + CommandHandler('reload_conf', self._reload_conf), CommandHandler('help', self._help), CommandHandler('version', self._version), ] @@ -300,6 +301,18 @@ class Telegram(RPC): (error, msg) = self.rpc_stop() self.send_msg(msg, bot=bot) + @authorized_only + def _reload_conf(self, bot: Bot, update: Update) -> None: + """ + Handler for /reload_conf. + Triggers a config file reload + :param bot: telegram bot + :param update: message update + :return: None + """ + msg = self.rpc_reload_conf() + self.send_msg(msg, bot=bot) + @authorized_only def _forcesell(self, bot: Bot, update: Update) -> None: """ diff --git a/freqtrade/state.py b/freqtrade/state.py index aaa5b4765..42bfb6e41 100644 --- a/freqtrade/state.py +++ b/freqtrade/state.py @@ -8,7 +8,8 @@ import enum class State(enum.Enum): """ - Bot running states + Bot application states """ RUNNING = 0 STOPPED = 1 + RELOAD_CONF = 2 diff --git a/freqtrade/tests/rpc/test_rpc_telegram.py b/freqtrade/tests/rpc/test_rpc_telegram.py index 2e6e3b285..0919455ad 100644 --- a/freqtrade/tests/rpc/test_rpc_telegram.py +++ b/freqtrade/tests/rpc/test_rpc_telegram.py @@ -70,12 +70,12 @@ def test_init(default_conf, mocker, caplog) -> None: assert start_polling.call_count == 0 # number of handles registered - assert start_polling.dispatcher.add_handler.call_count == 11 + assert start_polling.dispatcher.add_handler.call_count > 0 assert start_polling.start_polling.call_count == 1 message_str = "rpc.telegram is listening for following commands: [['status'], ['profit'], " \ "['balance'], ['start'], ['stop'], ['forcesell'], ['performance'], ['daily'], " \ - "['count'], ['help'], ['version']]" + "['count'], ['reload_conf'], ['help'], ['version']]" assert log_has(message_str, caplog.record_tuples) @@ -745,6 +745,29 @@ def test_stop_handle_already_stopped(default_conf, update, mocker) -> None: assert 'already stopped' in msg_mock.call_args_list[0][0][0] +def test_reload_conf_handle(default_conf, update, mocker) -> None: + """ Test _reload_conf() method """ + patch_coinmarketcap(mocker) + mocker.patch('freqtrade.freqtradebot.exchange.init', MagicMock()) + msg_mock = MagicMock() + mocker.patch.multiple( + 'freqtrade.rpc.telegram.Telegram', + _init=MagicMock(), + send_msg=msg_mock + ) + mocker.patch('freqtrade.freqtradebot.RPCManager', MagicMock()) + + freqtradebot = FreqtradeBot(default_conf) + telegram = Telegram(freqtradebot) + + freqtradebot.state = State.RUNNING + assert freqtradebot.state == State.RUNNING + telegram._reload_conf(bot=MagicMock(), update=update) + assert freqtradebot.state == State.RELOAD_CONF + assert msg_mock.call_count == 1 + assert 'Reloading config' in msg_mock.call_args_list[0][0][0] + + def test_forcesell_handle(default_conf, update, ticker, fee, ticker_sell_up, mocker) -> None: """ Test _forcesell() method diff --git a/freqtrade/tests/test_main.py b/freqtrade/tests/test_main.py index 82d8a8b34..9640a7350 100644 --- a/freqtrade/tests/test_main.py +++ b/freqtrade/tests/test_main.py @@ -3,12 +3,16 @@ Unit test file for main.py """ import logging +from copy import deepcopy from unittest.mock import MagicMock import pytest from freqtrade import OperationalException -from freqtrade.main import main, set_loggers +from freqtrade.arguments import Arguments +from freqtrade.freqtradebot import FreqtradeBot +from freqtrade.main import main, set_loggers, reconfigure +from freqtrade.state import State from freqtrade.tests.conftest import log_has @@ -140,3 +144,69 @@ def test_main_operational_exception(mocker, default_conf, caplog) -> None: main(args) assert log_has('Using config: config.json.example ...', caplog.record_tuples) assert log_has('Oh snap!', caplog.record_tuples) + + +def test_main_reload_conf(mocker, default_conf, caplog) -> None: + """ + Test main() function + In this test we are skipping the while True loop by throwing an exception. + """ + mocker.patch.multiple( + 'freqtrade.freqtradebot.FreqtradeBot', + _init_modules=MagicMock(), + worker=MagicMock(return_value=State.RELOAD_CONF), + cleanup=MagicMock(), + ) + mocker.patch( + 'freqtrade.configuration.Configuration._load_config_file', + lambda *args, **kwargs: default_conf + ) + mocker.patch('freqtrade.freqtradebot.CryptoToFiatConverter', MagicMock()) + mocker.patch('freqtrade.freqtradebot.RPCManager', MagicMock()) + + # Raise exception as side effect to avoid endless loop + reconfigure_mock = mocker.patch( + 'freqtrade.main.reconfigure', MagicMock(side_effect=Exception) + ) + + with pytest.raises(SystemExit): + main(['-c', 'config.json.example']) + + assert reconfigure_mock.call_count == 1 + assert log_has('Using config: config.json.example ...', caplog.record_tuples) + + +def test_reconfigure(mocker, default_conf) -> None: + """ Test recreate() function """ + mocker.patch.multiple( + 'freqtrade.freqtradebot.FreqtradeBot', + _init_modules=MagicMock(), + worker=MagicMock(side_effect=OperationalException('Oh snap!')), + cleanup=MagicMock(), + ) + mocker.patch( + 'freqtrade.configuration.Configuration._load_config_file', + lambda *args, **kwargs: default_conf + ) + mocker.patch('freqtrade.freqtradebot.CryptoToFiatConverter', MagicMock()) + mocker.patch('freqtrade.freqtradebot.RPCManager', MagicMock()) + + freqtrade = FreqtradeBot(default_conf) + + # Renew mock to return modified data + conf = deepcopy(default_conf) + conf['stake_amount'] += 1 + mocker.patch( + 'freqtrade.configuration.Configuration._load_config_file', + lambda *args, **kwargs: conf + ) + + # reconfigure should return a new instance + freqtrade2 = reconfigure( + freqtrade, + Arguments(['-c', 'config.json.example'], '').get_parsed_arg() + ) + + # Verify we have a new instance with the new config + assert freqtrade is not freqtrade2 + assert freqtrade.config['stake_amount'] + 1 == freqtrade2.config['stake_amount']