From e0d42d2eb7323acadbf221ae3c8b16b34fdac601 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 3 Jul 2021 17:03:12 +0200 Subject: [PATCH] Fix migrations, revert some parts related to amount properties --- freqtrade/persistence/migrations.py | 25 +++++---- freqtrade/persistence/models.py | 79 +++++++++++++++-------------- tests/test_persistence.py | 3 +- 3 files changed, 58 insertions(+), 49 deletions(-) diff --git a/freqtrade/persistence/migrations.py b/freqtrade/persistence/migrations.py index ef4a5623b..efadc7467 100644 --- a/freqtrade/persistence/migrations.py +++ b/freqtrade/persistence/migrations.py @@ -91,7 +91,8 @@ def migrate_trades_table(decl_base, inspector, engine, table_back_name: str, col stoploss_order_id, stoploss_last_update, max_rate, min_rate, sell_reason, sell_order_status, strategy, timeframe, open_trade_value, close_profit_abs, - leverage, borrowed, borrowed_currency, collateral_currency, interest_rate, liquidation_price, is_short + leverage, borrowed, borrowed_currency, collateral_currency, interest_rate, + liquidation_price, is_short ) select id, lower(exchange), case @@ -115,8 +116,8 @@ def migrate_trades_table(decl_base, inspector, engine, table_back_name: str, col {sell_order_status} sell_order_status, {strategy} strategy, {timeframe} timeframe, {open_trade_value} open_trade_value, {close_profit_abs} close_profit_abs, - {leverage} leverage, {borrowed} borrowed, {borrowed_currency} borrowed_currency, - {collateral_currency} collateral_currency, {interest_rate} interest_rate, + {leverage} leverage, {borrowed} borrowed, {borrowed_currency} borrowed_currency, + {collateral_currency} collateral_currency, {interest_rate} interest_rate, {liquidation_price} liquidation_price, {is_short} is_short from {table_back_name} """)) @@ -152,14 +153,17 @@ def migrate_orders_table(decl_base, inspector, engine, table_back_name: str, col # let SQLAlchemy create the schema as required decl_base.metadata.create_all(engine) + leverage = get_column_def(cols, 'leverage', 'null') + is_short = get_column_def(cols, 'is_short', 'False') with engine.begin() as connection: connection.execute(text(f""" insert into orders ( id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id, status, symbol, order_type, side, price, amount, filled, average, remaining, cost, - order_date, order_filled_date, order_update_date, leverage) + order_date, order_filled_date, order_update_date, leverage, is_short) select id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id, status, symbol, order_type, side, price, amount, filled, null average, remaining, cost, - order_date, order_filled_date, order_update_date, leverage + order_date, order_filled_date, order_update_date, + {leverage} leverage, {is_short} is_short from {table_back_name} """)) @@ -174,8 +178,9 @@ def check_migrate(engine, decl_base, previous_tables) -> None: tabs = get_table_names_for_table(inspector, 'trades') table_back_name = get_backup_name(tabs, 'trades_bak') - # Check for latest column - if not has_column(cols, 'open_trade_value'): + # Last added column of trades table + # To determine if migrations need to run + if not has_column(cols, 'collateral_currency'): logger.info(f'Running database migration for trades - backup: {table_back_name}') migrate_trades_table(decl_base, inspector, engine, table_back_name, cols) # Reread columns - the above recreated the table! @@ -188,9 +193,11 @@ def check_migrate(engine, decl_base, previous_tables) -> None: else: cols_order = inspector.get_columns('orders') - if not has_column(cols_order, 'average'): + # Last added column of order table + # To determine if migrations need to run + if not has_column(cols_order, 'leverage'): tabs = get_table_names_for_table(inspector, 'orders') # Empty for now - as there is only one iteration of the orders table so far. table_back_name = get_backup_name(tabs, 'orders_bak') - migrate_orders_table(decl_base, inspector, engine, table_back_name, cols) + migrate_orders_table(decl_base, inspector, engine, table_back_name, cols_order) diff --git a/freqtrade/persistence/models.py b/freqtrade/persistence/models.py index a974691be..8a52b4d4e 100644 --- a/freqtrade/persistence/models.py +++ b/freqtrade/persistence/models.py @@ -234,7 +234,7 @@ class LocalTrade(): close_profit: Optional[float] = None close_profit_abs: Optional[float] = None stake_amount: float = 0.0 - _amount: float = 0.0 + amount: float = 0.0 amount_requested: Optional[float] = None open_date: datetime close_date: Optional[datetime] = None @@ -266,8 +266,8 @@ class LocalTrade(): interest_rate: float = 0.0 liquidation_price: float = None is_short: bool = False - _borrowed: float = 0.0 - _leverage: float = None # * You probably want to use LocalTrade.leverage instead + borrowed: float = 0.0 + leverage: float = None # @property # def base_currency(self) -> str: @@ -275,42 +275,45 @@ class LocalTrade(): # raise OperationalException('LocalTrade.pair must be assigned') # return self.pair.split("/")[1] - @property - def amount(self) -> float: - if self._leverage is not None: - return self._amount * self.leverage - else: - return self._amount + # TODO: @samgermain: Amount should be persisted "as is". + # I've partially reverted this (this killed most of your tests) + # but leave this here as i'm not sure where you intended to use this. + # @property + # def amount(self) -> float: + # if self._leverage is not None: + # return self._amount * self.leverage + # else: + # return self._amount - @amount.setter - def amount(self, value): - self._amount = value + # @amount.setter + # def amount(self, value): + # self._amount = value - @property - def borrowed(self) -> float: - if self._leverage is not None: - if self.is_short: - # If shorting the full amount must be borrowed - return self._amount * self._leverage - else: - # If not shorting, then the trader already owns a bit - return self._amount * (self._leverage-1) - else: - return self._borrowed + # @property + # def borrowed(self) -> float: + # if self._leverage is not None: + # if self.is_short: + # # If shorting the full amount must be borrowed + # return self._amount * self._leverage + # else: + # # If not shorting, then the trader already owns a bit + # return self._amount * (self._leverage-1) + # else: + # return self._borrowed - @borrowed.setter - def borrowed(self, value): - self._borrowed = value - self._leverage = None + # @borrowed.setter + # def borrowed(self, value): + # self._borrowed = value + # self._leverage = None - @property - def leverage(self) -> float: - return self._leverage + # @property + # def leverage(self) -> float: + # return self._leverage - @leverage.setter - def leverage(self, value): - self._leverage = value - self._borrowed = None + # @leverage.setter + # def leverage(self, value): + # self._leverage = value + # self._borrowed = None # End of margin trading properties @@ -639,7 +642,7 @@ class LocalTrade(): # sec_per_day = Decimal(86400) sec_per_hour = Decimal(3600) total_seconds = Decimal((now - open_date).total_seconds()) - #days = total_seconds/sec_per_day or zero + # days = total_seconds/sec_per_day or zero hours = total_seconds/sec_per_hour or zero rate = Decimal(interest_rate or self.interest_rate) @@ -877,7 +880,7 @@ class Trade(_DECL_BASE, LocalTrade): close_profit = Column(Float) close_profit_abs = Column(Float) stake_amount = Column(Float, nullable=False) - _amount = Column(Float) + amount = Column(Float) amount_requested = Column(Float) open_date = Column(DateTime, nullable=False, default=datetime.utcnow) close_date = Column(DateTime) @@ -904,8 +907,8 @@ class Trade(_DECL_BASE, LocalTrade): timeframe = Column(Integer, nullable=True) # Margin trading properties - _leverage: float = None # * You probably want to use LocalTrade.leverage instead - _borrowed = Column(Float, nullable=False, default=0.0) + leverage = Column(Float, nullable=True) # TODO: can this be nullable, or should it default to 1? (must also be changed in migrations eventually) + borrowed = Column(Float, nullable=False, default=0.0) interest_rate = Column(Float, nullable=False, default=0.0) liquidation_price = Column(Float, nullable=True) is_short = Column(Boolean, nullable=False, default=False) diff --git a/tests/test_persistence.py b/tests/test_persistence.py index 358b59243..484a8739a 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -723,13 +723,11 @@ def test_migrate_new(mocker, default_conf, fee, caplog): order_date DATETIME, order_filled_date DATETIME, order_update_date DATETIME, - leverage FLOAT, PRIMARY KEY (id), CONSTRAINT _order_pair_order_id UNIQUE (ft_pair, order_id), FOREIGN KEY(ft_trade_id) REFERENCES trades (id) ) """)) - # TODO-mg @xmatthias: Had to add field leverage to this table, check that this is correct connection.execute(text(""" insert into orders ( id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id, status, symbol, order_type, side, price, amount, filled, remaining, cost, order_date, @@ -752,6 +750,7 @@ def test_migrate_new(mocker, default_conf, fee, caplog): assert orders[1].order_id == 'stop_order_id222' assert orders[1].ft_order_side == 'stoploss' + assert orders[0].is_short is False def test_migrate_mid_state(mocker, default_conf, fee, caplog):