From 9559f50eec5da4d9b070da248ea7ca09fccd5d52 Mon Sep 17 00:00:00 2001 From: gcarq Date: Tue, 20 Mar 2018 20:09:42 +0100 Subject: [PATCH 1/3] remove obsolete helper functions and make _state a public member. --- freqtrade/freqtradebot.py | 38 ++++++------------- freqtrade/rpc/rpc.py | 18 ++++----- freqtrade/tests/rpc/test_rpc.py | 28 +++++++------- freqtrade/tests/rpc/test_rpc_telegram.py | 48 ++++++++++++------------ freqtrade/tests/test_freqtradebot.py | 14 +++---- 5 files changed, 65 insertions(+), 81 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index e57f177e9..a312aed91 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -41,7 +41,7 @@ class FreqtradeBot(object): self.logger = Logger(name=__name__, level=config.get('loglevel')).get_logger() # Init bot states - self._state = State.STOPPED + self.state = State.STOPPED # Init objects self.config = config @@ -71,9 +71,9 @@ class FreqtradeBot(object): initial_state = self.config.get('initial_state') if initial_state: - self.update_state(State[initial_state.upper()]) + self.state = State[initial_state.upper()] else: - self.update_state(State.STOPPED) + self.state = State.STOPPED def clean(self) -> bool: """ @@ -82,41 +82,25 @@ class FreqtradeBot(object): """ self.rpc.send_msg('*Status:* `Stopping trader...`') self.logger.info('Stopping trader and cleaning up modules...') - self.update_state(State.STOPPED) + self.state = State.STOPPED self.rpc.cleanup() persistence.cleanup() return True - def update_state(self, state: State) -> None: - """ - Updates the application state - :param state: new state - :return: None - """ - self._state = state - - def get_state(self) -> State: - """ - Gets the current application state - :return: - """ - return self._state - def worker(self, old_state: None) -> State: """ Trading routine that must be run at each loop :param old_state: the previous service state from the previous call :return: current service state """ - new_state = self.get_state() # Log state transition - if new_state != old_state: - self.rpc.send_msg('*Status:* `{}`'.format(new_state.name.lower())) - self.logger.info('Changing state to: %s', new_state.name) + if self.state != old_state: + self.rpc.send_msg('*Status:* `{}`'.format(self.state.name.lower())) + self.logger.info('Changing state to: %s', self.state.name) - if new_state == State.STOPPED: + if self.state == State.STOPPED: time.sleep(1) - elif new_state == State.RUNNING: + elif self.state == State.RUNNING: min_secs = self.config.get('internals', {}).get( 'process_throttle_secs', Constants.PROCESS_THROTTLE_SECS @@ -130,7 +114,7 @@ class FreqtradeBot(object): self._throttle(func=self._process, min_secs=min_secs, nb_assets=nb_assets) - return new_state + return self.state def _throttle(self, func: Callable[..., Any], min_secs: float, *args, **kwargs) -> Any: """ @@ -196,7 +180,7 @@ class FreqtradeBot(object): ) ) self.logger.exception('OperationalException. Stopping trader ...') - self.update_state(State.STOPPED) + self.state = State.STOPPED return state_changed @cached(TTLCache(maxsize=1, ttl=1800)) diff --git a/freqtrade/rpc/rpc.py b/freqtrade/rpc/rpc.py index b4592f78a..db6ff69d1 100644 --- a/freqtrade/rpc/rpc.py +++ b/freqtrade/rpc/rpc.py @@ -41,7 +41,7 @@ class RPC(object): """ # Fetch open trade trades = Trade.query.filter(Trade.is_open.is_(True)).all() - if self.freqtrade.get_state() != State.RUNNING: + if self.freqtrade.state != State.RUNNING: return True, '*Status:* `trader is not running`' elif not trades: return True, '*Status:* `no active trade`' @@ -87,7 +87,7 @@ class RPC(object): def rpc_status_table(self) -> Tuple[bool, Any]: trades = Trade.query.filter(Trade.is_open.is_(True)).all() - if self.freqtrade.get_state() != State.RUNNING: + if self.freqtrade.state != State.RUNNING: return True, '*Status:* `trader is not running`' elif not trades: return True, '*Status:* `no active order`' @@ -285,18 +285,18 @@ class RPC(object): """ Handler for start. """ - if self.freqtrade.get_state() == State.RUNNING: + if self.freqtrade.state == State.RUNNING: return True, '*Status:* `already running`' - self.freqtrade.update_state(State.RUNNING) + self.freqtrade.state = State.RUNNING return False, '`Starting trader ...`' def rpc_stop(self) -> (bool, str): """ Handler for stop. """ - if self.freqtrade.get_state() == State.RUNNING: - self.freqtrade.update_state(State.STOPPED) + if self.freqtrade.state == State.RUNNING: + self.freqtrade.state = State.STOPPED return False, '`Stopping trader ...`' return True, '*Status:* `already stopped`' @@ -329,7 +329,7 @@ class RPC(object): self.freqtrade.execute_sell(trade, current_rate) # ---- EOF def _exec_forcesell ---- - if self.freqtrade.get_state() != State.RUNNING: + if self.freqtrade.state != State.RUNNING: return True, '`trader is not running`' if trade_id == 'all': @@ -357,7 +357,7 @@ class RPC(object): Handler for performance. Shows a performance statistic from finished trades """ - if self.freqtrade.get_state() != State.RUNNING: + if self.freqtrade.state != State.RUNNING: return True, '`trader is not running`' pair_rates = Trade.session.query(Trade.pair, @@ -378,7 +378,7 @@ class RPC(object): Returns the number of trades running :return: None """ - if self.freqtrade.get_state() != State.RUNNING: + if self.freqtrade.state != State.RUNNING: return True, '`trader is not running`' trades = Trade.query.filter(Trade.is_open.is_(True)).all() diff --git a/freqtrade/tests/rpc/test_rpc.py b/freqtrade/tests/rpc/test_rpc.py index 50943b1bc..84b06bee2 100644 --- a/freqtrade/tests/rpc/test_rpc.py +++ b/freqtrade/tests/rpc/test_rpc.py @@ -41,12 +41,12 @@ def test_rpc_trade_status(default_conf, ticker, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) rpc = RPC(freqtradebot) - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED (error, result) = rpc.rpc_trade_status() assert error assert 'trader is not running' in result - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING (error, result) = rpc.rpc_trade_status() assert error assert 'no active trade' in result @@ -89,12 +89,12 @@ def test_rpc_status_table(default_conf, ticker, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) rpc = RPC(freqtradebot) - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED (error, result) = rpc.rpc_status_table() assert error assert '*Status:* `trader is not running`' in result - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING (error, result) = rpc.rpc_status_table() assert error assert '*Status:* `no active order`' in result @@ -344,17 +344,17 @@ def test_rpc_start(mocker, default_conf) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) rpc = RPC(freqtradebot) - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED (error, result) = rpc.rpc_start() assert not error assert '`Starting trader ...`' in result - assert freqtradebot.get_state() == State.RUNNING + assert freqtradebot.state == State.RUNNING (error, result) = rpc.rpc_start() assert error assert '*Status:* `already running`' in result - assert freqtradebot.get_state() == State.RUNNING + assert freqtradebot.state == State.RUNNING def test_rpc_stop(mocker, default_conf) -> None: @@ -372,17 +372,17 @@ def test_rpc_stop(mocker, default_conf) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) rpc = RPC(freqtradebot) - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING (error, result) = rpc.rpc_stop() assert not error assert '`Stopping trader ...`' in result - assert freqtradebot.get_state() == State.STOPPED + assert freqtradebot.state == State.STOPPED (error, result) = rpc.rpc_stop() assert error assert '*Status:* `already stopped`' in result - assert freqtradebot.get_state() == State.STOPPED + assert freqtradebot.state == State.STOPPED def test_rpc_forcesell(default_conf, ticker, mocker) -> None: @@ -410,12 +410,12 @@ def test_rpc_forcesell(default_conf, ticker, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) rpc = RPC(freqtradebot) - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED (error, res) = rpc.rpc_forcesell(None) assert error assert res == '`trader is not running`' - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING (error, res) = rpc.rpc_forcesell(None) assert error assert res == 'Invalid argument.' @@ -433,7 +433,7 @@ def test_rpc_forcesell(default_conf, ticker, mocker) -> None: assert not error assert res == '' - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED (error, res) = rpc.rpc_forcesell(None) assert error assert res == '`trader is not running`' @@ -442,7 +442,7 @@ def test_rpc_forcesell(default_conf, ticker, mocker) -> None: assert error assert res == '`trader is not running`' - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING assert cancel_order_mock.call_count == 0 # make an limit-buy open trade mocker.patch( diff --git a/freqtrade/tests/rpc/test_rpc_telegram.py b/freqtrade/tests/rpc/test_rpc_telegram.py index 4796b077e..86dfd6f41 100644 --- a/freqtrade/tests/rpc/test_rpc_telegram.py +++ b/freqtrade/tests/rpc/test_rpc_telegram.py @@ -301,13 +301,13 @@ def test_status_handle(default_conf, update, ticker, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) telegram = Telegram(freqtradebot) - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED telegram._status(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 assert 'trader is not running' in msg_mock.call_args_list[0][0][0] msg_mock.reset_mock() - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING telegram._status(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 assert 'no active trade' in msg_mock.call_args_list[0][0][0] @@ -347,13 +347,13 @@ def test_status_table_handle(default_conf, update, ticker, mocker) -> None: freqtradebot = FreqtradeBot(conf, create_engine('sqlite://')) telegram = Telegram(freqtradebot) - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED telegram._status_table(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 assert 'trader is not running' in msg_mock.call_args_list[0][0][0] msg_mock.reset_mock() - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING telegram._status_table(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 assert 'no active order' in msg_mock.call_args_list[0][0][0] @@ -470,7 +470,7 @@ def test_daily_wrong_input(default_conf, update, ticker, mocker) -> None: # Try invalid data msg_mock.reset_mock() - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING update.message.text = '/daily -2' telegram._daily(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 @@ -478,7 +478,7 @@ def test_daily_wrong_input(default_conf, update, ticker, mocker) -> None: # Try invalid data msg_mock.reset_mock() - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING update.message.text = '/daily today' telegram._daily(bot=MagicMock(), update=update) assert str('Daily Profit over the last 7 days') in msg_mock.call_args_list[0][0][0] @@ -665,10 +665,10 @@ def test_start_handle(default_conf, update, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) telegram = Telegram(freqtradebot) - freqtradebot.update_state(State.STOPPED) - assert freqtradebot.get_state() == State.STOPPED + freqtradebot.state = State.STOPPED + assert freqtradebot.state == State.STOPPED telegram._start(bot=MagicMock(), update=update) - assert freqtradebot.get_state() == State.RUNNING + assert freqtradebot.state == State.RUNNING assert msg_mock.call_count == 0 @@ -689,10 +689,10 @@ def test_start_handle_already_running(default_conf, update, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) telegram = Telegram(freqtradebot) - freqtradebot.update_state(State.RUNNING) - assert freqtradebot.get_state() == State.RUNNING + freqtradebot.state = State.RUNNING + assert freqtradebot.state == State.RUNNING telegram._start(bot=MagicMock(), update=update) - assert freqtradebot.get_state() == State.RUNNING + assert freqtradebot.state == State.RUNNING assert msg_mock.call_count == 1 assert 'already running' in msg_mock.call_args_list[0][0][0] @@ -714,10 +714,10 @@ def test_stop_handle(default_conf, update, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) telegram = Telegram(freqtradebot) - freqtradebot.update_state(State.RUNNING) - assert freqtradebot.get_state() == State.RUNNING + freqtradebot.state = State.RUNNING + assert freqtradebot.state == State.RUNNING telegram._stop(bot=MagicMock(), update=update) - assert freqtradebot.get_state() == State.STOPPED + assert freqtradebot.state == State.STOPPED assert msg_mock.call_count == 1 assert 'Stopping trader' in msg_mock.call_args_list[0][0][0] @@ -739,10 +739,10 @@ def test_stop_handle_already_stopped(default_conf, update, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) telegram = Telegram(freqtradebot) - freqtradebot.update_state(State.STOPPED) - assert freqtradebot.get_state() == State.STOPPED + freqtradebot.state = State.STOPPED + assert freqtradebot.state == State.STOPPED telegram._stop(bot=MagicMock(), update=update) - assert freqtradebot.get_state() == State.STOPPED + assert freqtradebot.state == State.STOPPED assert msg_mock.call_count == 1 assert 'already stopped' in msg_mock.call_args_list[0][0][0] @@ -881,7 +881,7 @@ def test_forcesell_handle_invalid(default_conf, update, mocker) -> None: telegram = Telegram(freqtradebot) # Trader is not running - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED update.message.text = '/forcesell 1' telegram._forcesell(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 @@ -889,7 +889,7 @@ def test_forcesell_handle_invalid(default_conf, update, mocker) -> None: # No argument msg_mock.reset_mock() - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING update.message.text = '/forcesell' telegram._forcesell(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 @@ -897,7 +897,7 @@ def test_forcesell_handle_invalid(default_conf, update, mocker) -> None: # Invalid argument msg_mock.reset_mock() - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING update.message.text = '/forcesell 123456' telegram._forcesell(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 @@ -962,7 +962,7 @@ def test_performance_handle_invalid(default_conf, update, mocker) -> None: telegram = Telegram(freqtradebot) # Trader is not running - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED telegram._performance(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 assert 'not running' in msg_mock.call_args_list[0][0][0] @@ -989,12 +989,12 @@ def test_count_handle(default_conf, update, ticker, mocker) -> None: freqtradebot = FreqtradeBot(default_conf, create_engine('sqlite://')) telegram = Telegram(freqtradebot) - freqtradebot.update_state(State.STOPPED) + freqtradebot.state = State.STOPPED telegram._count(bot=MagicMock(), update=update) assert msg_mock.call_count == 1 assert 'not running' in msg_mock.call_args_list[0][0][0] msg_mock.reset_mock() - freqtradebot.update_state(State.RUNNING) + freqtradebot.state = State.RUNNING # Create some test data freqtradebot.create_trade() diff --git a/freqtrade/tests/test_freqtradebot.py b/freqtrade/tests/test_freqtradebot.py index d58b428da..608200071 100644 --- a/freqtrade/tests/test_freqtradebot.py +++ b/freqtrade/tests/test_freqtradebot.py @@ -104,12 +104,12 @@ def test_freqtradebot(mocker, default_conf) -> None: Test __init__, _init_modules, update_state, and get_state methods """ freqtrade = get_patched_freqtradebot(mocker, default_conf) - assert freqtrade.get_state() is State.RUNNING + assert freqtrade.state is State.RUNNING conf = deepcopy(default_conf) conf.pop('initial_state') freqtrade = FreqtradeBot(conf) - assert freqtrade.get_state() is State.STOPPED + assert freqtrade.state is State.STOPPED def test_clean(mocker, default_conf, caplog) -> None: @@ -120,10 +120,10 @@ def test_clean(mocker, default_conf, caplog) -> None: mocker.patch('freqtrade.persistence.cleanup', mock_cleanup) freqtrade = get_patched_freqtradebot(mocker, default_conf) - assert freqtrade.get_state() == State.RUNNING + assert freqtrade.state == State.RUNNING assert freqtrade.clean() - assert freqtrade.get_state() == State.STOPPED + assert freqtrade.state == State.STOPPED assert log_has('Stopping trader and cleaning up modules...', caplog.record_tuples) assert mock_cleanup.call_count == 1 @@ -152,7 +152,7 @@ def test_worker_stopped(mocker, default_conf, caplog) -> None: mock_sleep = mocker.patch('time.sleep', return_value=None) freqtrade = get_patched_freqtradebot(mocker, default_conf) - freqtrade.update_state(State.STOPPED) + freqtrade.state = State.STOPPED state = freqtrade.worker(old_state=State.RUNNING) assert state is State.STOPPED assert log_has('Changing state to: STOPPED', caplog.record_tuples) @@ -472,11 +472,11 @@ def test_process_operational_exception(default_conf, ticker, health, mocker) -> buy=MagicMock(side_effect=OperationalException) ) freqtrade = FreqtradeBot(default_conf, create_engine('sqlite://')) - assert freqtrade.get_state() == State.RUNNING + assert freqtrade.state == State.RUNNING result = freqtrade._process() assert result is False - assert freqtrade.get_state() == State.STOPPED + assert freqtrade.state == State.STOPPED assert 'OperationalException' in msg_mock.call_args_list[-1][0][0] From 9df5e09a82bb4ac4bae4b92fdf5431052c828642 Mon Sep 17 00:00:00 2001 From: gcarq Date: Tue, 20 Mar 2018 20:36:03 +0100 Subject: [PATCH 2/3] remove function assertions --- freqtrade/tests/test_freqtradebot.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/freqtrade/tests/test_freqtradebot.py b/freqtrade/tests/test_freqtradebot.py index 608200071..cd271d273 100644 --- a/freqtrade/tests/test_freqtradebot.py +++ b/freqtrade/tests/test_freqtradebot.py @@ -85,8 +85,6 @@ def test_freqtradebot_object() -> None: Test the FreqtradeBot object has the mandatory public methods """ assert hasattr(FreqtradeBot, 'worker') - assert hasattr(FreqtradeBot, 'get_state') - assert hasattr(FreqtradeBot, 'update_state') assert hasattr(FreqtradeBot, 'clean') assert hasattr(FreqtradeBot, 'create_trade') assert hasattr(FreqtradeBot, 'get_target_bid') From b8f322d8f67fad300bbe4817f452b1dc2ef8b3aa Mon Sep 17 00:00:00 2001 From: gcarq Date: Wed, 21 Mar 2018 19:27:30 +0100 Subject: [PATCH 3/3] revert worker() changes --- freqtrade/freqtradebot.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index a312aed91..29790515e 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -94,13 +94,14 @@ class FreqtradeBot(object): :return: current service state """ # Log state transition - if self.state != old_state: - self.rpc.send_msg('*Status:* `{}`'.format(self.state.name.lower())) - self.logger.info('Changing state to: %s', self.state.name) + state = self.state + if state != old_state: + self.rpc.send_msg('*Status:* `{}`'.format(state.name.lower())) + self.logger.info('Changing state to: %s', state.name) - if self.state == State.STOPPED: + if state == State.STOPPED: time.sleep(1) - elif self.state == State.RUNNING: + elif state == State.RUNNING: min_secs = self.config.get('internals', {}).get( 'process_throttle_secs', Constants.PROCESS_THROTTLE_SECS @@ -114,7 +115,7 @@ class FreqtradeBot(object): self._throttle(func=self._process, min_secs=min_secs, nb_assets=nb_assets) - return self.state + return state def _throttle(self, func: Callable[..., Any], min_secs: float, *args, **kwargs) -> Any: """