refactoring and testing bot_id

freqtrade/persistence.py

@@ -53,11 +53,12 @@ def init(config: Dict) -> None:
     Trade.session = session()
     Trade.query = session.query_property()
     _DECL_BASE.metadata.create_all(engine)
-    check_migrate(engine)
+    bot_id = config.get('bot_id', '0')
+    check_migrate(engine, bot_id)
 
     # Clean dry_run DB if the db is not in-memory
     if config.get('dry_run', False) and db_url != 'sqlite://':
-        clean_dry_run_db(config.get('bot_id', 0))
+        clean_dry_run_db(bot_id)
 
 
 def has_column(columns, searchname: str) -> bool:
@@ -68,12 +69,34 @@ def get_column_def(columns, column: str, default: str) -> str:
     return default if not has_column(columns, column) else column
 
 
-def check_migrate(engine) -> None:
+def check_migrate(engine, bot_id_value) -> None:
     """
     Checks if migration is necessary and migrates if necessary
     """
     inspector = inspect(engine)
 
+    cols = inspector.get_columns('trades')
+
+    needs_migration = False
+
+    if not has_column(cols, 'bot_id'):
+        engine.execute("alter table trades add bot_id integer")
+        engine.execute("create index bot_id_idx ON trades (bot_id);")
+        needs_migration = True
+
+    # Check for latest column
+    if not has_column(cols, 'ticker_interval'):
+        needs_migration = True
+
+    if needs_migration:
+        migrate(engine, bot_id_value)
+
+
+def migrate(engine, bot_id_str) -> None:
+    """
+    migrate trades table
+    """
+    inspector = inspect(engine)
     cols = inspector.get_columns('trades')
     tabs = inspector.get_table_names()
     table_back_name = 'trades_bak'
@@ -81,8 +104,6 @@ def check_migrate(engine) -> None:
         table_back_name = f'trades_bak{i}'
         logger.info(f'trying {table_back_name}')
 
-    # Check for latest column
-    if not has_column(cols, 'ticker_interval'):
     fee_open = get_column_def(cols, 'fee_open', 'fee')
     fee_close = get_column_def(cols, 'fee_close', 'fee')
     open_rate_requested = get_column_def(cols, 'open_rate_requested', 'null')
@@ -93,6 +114,7 @@ def check_migrate(engine) -> None:
     sell_reason = get_column_def(cols, 'sell_reason', 'null')
     strategy = get_column_def(cols, 'strategy', 'null')
     ticker_interval = get_column_def(cols, 'ticker_interval', 'null')
+    bot_id = get_column_def(cols, 'bot_id', bot_id_str)
 
     # Schema migration necessary
     engine.execute(f"alter table trades rename to {table_back_name}")
@@ -101,13 +123,13 @@ def check_migrate(engine) -> None:
 
     # Copy data back - following the correct schema
     engine.execute(f"""insert into trades
-                (id, exchange, pair, is_open, fee_open, fee_close, open_rate,
+            (id, bot_id, exchange, pair, is_open, fee_open, fee_close, open_rate,
             open_rate_requested, close_rate, close_rate_requested, close_profit,
             stake_amount, amount, open_date, close_date, open_order_id,
             stop_loss, initial_stop_loss, max_rate, sell_reason, strategy,
             ticker_interval
             )
-            select id, lower(exchange),
+        select id, {bot_id} bot_id, lower(exchange),
             case
                 when instr(pair, '_') != 0 then
                 substr(pair,    instr(pair, '_') + 1) || '/' ||
@@ -129,21 +151,6 @@ def check_migrate(engine) -> None:
     inspector = inspect(engine)
     cols = inspector.get_columns('trades')
 
-        # backwards compatible
-        if not has_column(cols, 'open_rate_requested'):
-            engine.execute("alter table trades add open_rate_requested float")
-        if not has_column(cols, 'close_rate_requested'):
-            engine.execute("alter table trades add close_rate_requested float")
-        if not has_column(cols, 'stop_loss'):
-            engine.execute("alter table trades add stop_loss float")
-        if not has_column(cols, 'initial_stop_loss'):
-            engine.execute("alter table trades add initial_stop_loss float")
-        if not has_column(cols, 'max_rate'):
-            engine.execute("alter table trades add max_rate float")
-        if not has_column(cols, 'bot_id'):
-            engine.execute("alter table trades add bot_id integer")
-            engine.execute("create index bot_id_idx ON trades (bot_id);")
-
 
 def cleanup() -> None:
     """

freqtrade/tests/test_persistence.py

@@ -576,3 +576,63 @@ def test_adjust_stop_loss(limit_buy_order, limit_sell_order, fee):
     assert round(trade.stop_loss, 8) == 1.26
     assert trade.max_rate == 1.4
     assert trade.initial_stop_loss == 0.95
+
+
+def test_migrate_bot_id(mocker, default_conf, fee, caplog):
+    """
+    Test Database migration (starting with new pairformat)
+    migration will put bot_id
+    """
+    amount = 103.223
+    # Always create all columns apart from the last!
+    create_table_old = """CREATE TABLE IF NOT EXISTS "trades" (
+                                id INTEGER NOT NULL,
+                                exchange VARCHAR NOT NULL,
+                                pair VARCHAR NOT NULL,
+                                is_open BOOLEAN NOT NULL,
+                                fee FLOAT NOT NULL,
+                                open_rate FLOAT,
+                                close_rate FLOAT,
+                                close_profit FLOAT,
+                                stake_amount FLOAT NOT NULL,
+                                amount FLOAT,
+                                open_date DATETIME NOT NULL,
+                                close_date DATETIME,
+                                open_order_id VARCHAR,
+                                stop_loss FLOAT,
+                                initial_stop_loss FLOAT,
+                                max_rate FLOAT,
+                                sell_reason VARCHAR,
+                                strategy VARCHAR,
+                                PRIMARY KEY (id),
+                                CHECK (is_open IN (0, 1))
+                                );"""
+    insert_table_old = """INSERT INTO trades (exchange, pair, is_open, fee,
+                          open_rate, stake_amount, amount, open_date,
+                          stop_loss, initial_stop_loss, max_rate)
+                          VALUES ('binance', 'ETC/BTC', 1, {fee},
+                          0.00258580, {stake}, {amount},
+                          '2019-11-28 12:44:24.000000',
+                          0.0, 0.0, 0.0)
+                          """.format(fee=fee.return_value,
+                                     stake=default_conf.get("stake_amount"),
+                                     amount=amount
+                                     )
+    engine = create_engine('sqlite://')
+    mocker.patch('freqtrade.persistence.create_engine', lambda *args, **kwargs: engine)
+
+    # Create table using the old format
+    engine.execute(create_table_old)
+    engine.execute(insert_table_old)
+
+    # fake previous backup
+    # engine.execute("create table trades_bak as select * from trades")
+
+    # engine.execute("create table trades_bak1 as select * from trades")
+    # Run init to test migration
+    default_conf['bot_id'] = 1
+    init(default_conf)
+
+    assert len(Trade.query.filter(Trade.id == 1).all()) == 1
+    trade = Trade.query.filter(Trade.id == 1).first()
+    assert trade.bot_id == 1