Repository: liampauling/flumine
Pull request #385 (2021-03-02 10:59:34)

incorrect handling of replace on runner context (adds to live trade c…
…ount)


Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index 48a96b8..19055e8 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -3,6 +3,17 @@
 Release History
 ---------------
 
+1.17.6 (2021-03-05)
++++++++++++++++++++
+
+**Improvements**
+
+- trade id added to context to prevent race condition and better visibility on live trades
+
+**Bug Fixes**
+
+- incorrect handling of replace on runner context fix (adds to live trade count)
+
 1.17.5 (2021-03-01)
 +++++++++++++++++++
 
diff --git a/flumine/__version__.py b/flumine/__version__.py
index 5ef6e16..9cef1e8 100644
--- a/flumine/__version__.py
+++ b/flumine/__version__.py
@@ -1,6 +1,6 @@
 __title__ = "flumine"
 __description__ = "Betfair trading framework"
 __url__ = "https://github.com/liampauling/flumine"
-__version__ = "1.17.5"
+__version__ = "1.17.6"
 __author__ = "Liam Pauling"
 __license__ = "MIT"
diff --git a/flumine/execution/transaction.py b/flumine/execution/transaction.py
index a3c09ab..fad0068 100644
--- a/flumine/execution/transaction.py
+++ b/flumine/execution/transaction.py
@@ -46,6 +46,9 @@ class Transaction:
             return False
         # place
         order.place(self.market.market_book.publish_time)
+        if execute:
+            runner_context = order.trade.strategy.get_runner_context(*order.lookup)
+            runner_context.place(order.trade.id)
         if order.id not in self.market.blotter:
             self.market.blotter[order.id] = order
             if order.trade.market_notes is None:
diff --git a/flumine/order/order.py b/flumine/order/order.py
index 554d598..972e202 100644
--- a/flumine/order/order.py
+++ b/flumine/order/order.py
@@ -299,8 +299,6 @@ class BetfairOrder(BaseOrder):
     def place(self, publish_time: int) -> None:
         self.publish_time = publish_time
         self.placing()
-        runner_context = self.trade.strategy.get_runner_context(*self.lookup)
-        runner_context.place()
 
     def cancel(self, size_reduction: float = None) -> None:
         if self.order_type.ORDER_TYPE == OrderTypes.LIMIT:
diff --git a/flumine/order/trade.py b/flumine/order/trade.py
index 2922710..1e65c2f 100644
--- a/flumine/order/trade.py
+++ b/flumine/order/trade.py
@@ -74,7 +74,7 @@ class Trade:
         runner_context = self.strategy.get_runner_context(
             self.market_id, self.selection_id, self.handicap
         )
-        runner_context.reset()  # todo race condition?
+        runner_context.reset(self.id)
 
     @property
     def complete(self) -> bool:
diff --git a/flumine/strategy/runnercontext.py b/flumine/strategy/runnercontext.py
index 3f9108e..854e39e 100644
--- a/flumine/strategy/runnercontext.py
+++ b/flumine/strategy/runnercontext.py
@@ -1,6 +1,9 @@
+import logging
 import datetime
 from typing import Optional
 
+logger = logging.getLogger(__name__)
+
 
 class RunnerContext:
     """Runner context at strategy level"""
@@ -10,26 +13,40 @@ class RunnerContext:
         self.invested = False
         self.datetime_last_placed = None
         self.datetime_last_reset = None
-        self.trade_count = 0
-        self.live_trade_count = 0
+        self._trades = []
+        self._live_trades = []
 
-    def place(self) -> None:
+    def place(self, trade_id) -> None:
         self.invested = True
         self.datetime_last_placed = datetime.datetime.utcnow()
-        self.trade_count += 1
-        self.live_trade_count += 1
+        self._trades.append(trade_id)
+        self._live_trades.append(trade_id)
 
-    def reset(self) -> None:
+    def reset(self, trade_id) -> None:
         self.datetime_last_reset = datetime.datetime.utcnow()
-        self.live_trade_count -= 1
+        try:
+            self._live_trades.remove(trade_id)
+        except ValueError:
+            logger.warning(
+                "Trade '%s' not present in RunnerContext live trades on reset"
+                % trade_id
+            )
 
     @property
     def executable_orders(self) -> bool:
-        if self.live_trade_count:
+        if self._live_trades:
             return True
         else:
             return False
 
+    @property
+    def trade_count(self) -> int:
+        return len(self._trades)
+
+    @property
+    def live_trade_count(self) -> int:
+        return len(self._live_trades)
+
     @property
     def placed_elapsed_seconds(self) -> Optional[float]:
         if self.datetime_last_placed:


Test patch:
diff --git a/tests/test_order.py b/tests/test_order.py
index 0014e06..c6aa74f 100644
--- a/tests/test_order.py
+++ b/tests/test_order.py
@@ -274,9 +274,6 @@ class BetfairOrderTest(unittest.TestCase):
         self.order.place(123)
         mock_placing.assert_called_with()
         self.assertEqual(self.order.publish_time, 123)
-        self.order.trade.strategy.get_runner_context.assert_called_with(
-            *self.order.lookup
-        )
 
     @mock.patch(
         "flumine.order.order.BetfairOrder.size_remaining",
diff --git a/tests/test_runnercontext.py b/tests/test_runnercontext.py
index 3649e24..7e952e1 100644
--- a/tests/test_runnercontext.py
+++ b/tests/test_runnercontext.py
@@ -1,3 +1,4 @@
+import uuid
 import unittest
 import datetime
 
@@ -13,27 +14,45 @@ class RunnerContextTest(unittest.TestCase):
         self.assertFalse(self.context.invested)
         self.assertIsNone(self.context.datetime_last_placed)
         self.assertIsNone(self.context.datetime_last_reset)
-        self.assertEqual(self.context.trade_count, 0)
-        self.assertEqual(self.context.live_trade_count, 0)
+        self.assertEqual(self.context._trades, [])
+        self.assertEqual(self.context._live_trades, [])
 
     def test_place(self):
-        self.context.place()
+        id_ = uuid.uuid1()
+        self.context.place(id_)
         self.assertTrue(self.context.invested)
         self.assertIsNotNone(self.context.datetime_last_placed)
-        self.assertEqual(self.context.trade_count, 1)
-        self.assertEqual(self.context.live_trade_count, 1)
+        self.assertEqual(self.context._trades, [id_])
+        self.assertEqual(self.context._live_trades, [id_])
 
     def test_reset(self):
-        self.context.live_trade_count = 1
-        self.context.reset()
+        id_ = uuid.uuid1()
+        self.context._live_trades = [id_]
+        self.context.reset(id_)
         self.assertIsNotNone(self.context.datetime_last_reset)
-        self.assertEqual(self.context.live_trade_count, 0)
+        self.assertEqual(self.context._live_trades, [])
+
+    def test_reset_error(self):
+        id_ = uuid.uuid1()
+        self.context.reset(id_)
+        self.assertIsNotNone(self.context.datetime_last_reset)
+        self.assertEqual(self.context._live_trades, [])
 
     def test_executable_orders(self):
         self.assertFalse(self.context.executable_orders)
-        self.context.live_trade_count = 1
+        self.context._live_trades = ["test"]
         self.assertTrue(self.context.executable_orders)
 
+    def test_trade_count(self):
+        self.assertEqual(self.context.trade_count, 0)
+        self.context._trades = ["test"]
+        self.assertEqual(self.context.trade_count, 1)
+
+    def test_live_trade_count(self):
+        self.assertEqual(self.context.live_trade_count, 0)
+        self.context._live_trades = ["test"]
+        self.assertEqual(self.context.live_trade_count, 1)
+
     def test_placed_elapsed_seconds(self):
         self.assertIsNone(self.context.placed_elapsed_seconds)
         self.context.datetime_last_placed = datetime.datetime.utcnow()
diff --git a/tests/test_trade.py b/tests/test_trade.py
index 939ca85..51234c7 100644
--- a/tests/test_trade.py
+++ b/tests/test_trade.py
@@ -71,7 +71,7 @@ class TradeTest(unittest.TestCase):
         runner_context = self.mock_strategy.get_runner_context(
             self.trade.market_id, self.trade.selection_id, self.trade.handicap
         )
-        runner_context.reset.assert_called_with()
+        runner_context.reset.assert_called_with(self.trade.id)
         self.assertIsNotNone(self.trade.date_time_complete)
 
     def test_complete(self):
diff --git a/tests/test_transaction.py b/tests/test_transaction.py
index 533c771..21fd7e7 100644
--- a/tests/test_transaction.py
+++ b/tests/test_transaction.py
@@ -28,7 +28,7 @@ class TransactionTest(unittest.TestCase):
     @mock.patch("flumine.execution.transaction.events")
     def test_place_order(self, mock_events, mock__validate_controls):
         self.transaction.market.blotter = {}
-        mock_order = mock.Mock(id="123")
+        mock_order = mock.Mock(id="123", lookup=(1, 2, 3))
         mock_order.trade.market_notes = None
         self.assertTrue(self.transaction.place_order(mock_order))
         mock_order.place.assert_called_with(
@@ -42,6 +42,9 @@ class TransactionTest(unittest.TestCase):
         mock__validate_controls.assert_called_with(mock_order, OrderPackageType.PLACE)
         self.transaction._pending_place = [(mock_order, None)]
         self.assertTrue(self.transaction._pending_orders)
+        mock_order.trade.strategy.get_runner_context.assert_called_with(
+            *mock_order.lookup
+        )
 
     @mock.patch(
         "flumine.execution.transaction.Transaction._validate_controls",
@@ -66,7 +69,7 @@ class TransactionTest(unittest.TestCase):
         return_value=True,
     )
     def test_place_order_retry(self, mock__validate_controls):
-        mock_order = mock.Mock()
+        mock_order = mock.Mock(lookup=(1, 2, 3))
         self.transaction.market.blotter = {mock_order.id: mock_order}
         with self.assertRaises(OrderError):
             self.transaction.place_order(mock_order)