Repository: liampauling/flumine
Pull request #298 (2020-10-26 11:08:37)

add violation msg to order on violation
return bool on place order based on validation


Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index e985ddb..604e15e 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -3,20 +3,6 @@
 Release History
 ---------------
 
-1.14.1 (2020-10-29)
-+++++++++++++++++++
-
-**Improvements**
-
-- #297 add violation msg to order on violation
-- Graceful worker shutdown
-- Terminate worker example added
-
-**Libraries**
-
-- betfairlightweight upgraded to 2.9.2
-- python-json-logger upgraded to 2.0.1
-
 1.14.0 (2020-10-12)
 +++++++++++++++++++
 
diff --git a/examples/workers/inplayservice.py b/examples/workers/inplayservice.py
index 758e819..569e2d6 100644
--- a/examples/workers/inplayservice.py
+++ b/examples/workers/inplayservice.py
@@ -4,11 +4,6 @@ from flumine.events.events import CustomEvent
 
 logger = logging.getLogger(__name__)
 
-"""
-See examples/tennisexample.py in how
-to use this worker for betting.
-"""
-
 
 def poll_in_play_service(context: dict, flumine, event_type_id: str) -> None:
     trading = flumine.client.betting_client
diff --git a/examples/workers/terminate.py b/examples/workers/terminate.py
deleted file mode 100644
index 09094ad..0000000
--- a/examples/workers/terminate.py
+++ /dev/null
@@ -1,48 +0,0 @@
-import logging
-import datetime
-from flumine.events.events import TerminationEvent
-
-logger = logging.getLogger(__name__)
-
-"""
-Worker can be used as followed:
-
-    framework.add_worker(
-        BackgroundWorker(
-            framework,
-            terminate,
-            func_kwargs={"today_only": True, "seconds_closed": 1200},
-            interval=60,
-            start_delay=60,
-        )
-    )
-
-This will run every 60s and will terminate 
-the framework if all markets starting 'today' 
-have been closed for at least 1200s
-"""
-
-
-def terminate(
-    context: dict, flumine, today_only: bool = True, seconds_closed: int = 600
-) -> None:
-    """terminate framework if no markets
-    live today.
-    """
-    markets = list(flumine.markets.markets.values())
-    markets_today = [
-        m
-        for m in markets
-        if m.market_start_datetime.date() == datetime.datetime.utcnow().date()
-        and (
-            m.elapsed_seconds_closed is None
-            or (m.elapsed_seconds_closed and m.elapsed_seconds_closed < seconds_closed)
-        )
-    ]
-    if today_only:
-        market_count = len(markets_today)
-    else:
-        market_count = len(markets)
-    if market_count == 0:
-        logger.info("No more markets available, terminating framework")
-        flumine.handler_queue.put(TerminationEvent(None))
diff --git a/flumine/__version__.py b/flumine/__version__.py
index d8848ae..303d19c 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.14.1"
+__version__ = "1.14.0"
 __author__ = "Liam Pauling"
 __license__ = "MIT"
diff --git a/flumine/baseflumine.py b/flumine/baseflumine.py
index 555fe83..ef3c260 100644
--- a/flumine/baseflumine.py
+++ b/flumine/baseflumine.py
@@ -370,9 +370,7 @@ class BaseFlumine:
         # shutdown thread pools
         self.simulated_execution.shutdown()
         self.betfair_execution.shutdown()
-        # shutdown workers
-        for w in self._workers:
-            w.shutdown()
+        # todo shutdown workers
         # shutdown logging controls
         self.log_control(events.TerminationEvent(None))
         for c in self._logging_controls:
diff --git a/flumine/controls/__init__.py b/flumine/controls/__init__.py
index ba13cd9..82720c3 100644
--- a/flumine/controls/__init__.py
+++ b/flumine/controls/__init__.py
@@ -20,8 +20,9 @@ class BaseControl:
         raise NotImplementedError
 
     def _on_error(self, order: BaseOrder, error: str) -> None:
-        order.violation()
+        violation_msg = "Order has violated: {0} Error: {1}".format(self.NAME, error)
+        order.violation(violation_msg)
         logger.warning(
-            "Order has violated {0} and will not be placed".format(self.NAME),
+            violation_msg,
             extra={"control": self.NAME, "error": error, "order": order.info},
         )
diff --git a/flumine/order/order.py b/flumine/order/order.py
index 9eb0b62..8521853 100644
--- a/flumine/order/order.py
+++ b/flumine/order/order.py
@@ -59,6 +59,7 @@ class BaseOrder:
         self.runner_status = None  # RunnerBook.status
         self.status = None
         self.status_log = []
+        self.violation_msg = None
 
         self.bet_id = None
         self.update_data = {}  # stores cancel/update/replace data
@@ -110,8 +111,9 @@ class BaseOrder:
         self._update_status(OrderStatus.VOIDED)
         self.update_data.clear()
 
-    def violation(self) -> None:
+    def violation(self, violation_msg: str) -> None:
         self._update_status(OrderStatus.VIOLATION)
+        self.violation_msg = violation_msg
         self.update_data.clear()
 
     # updates
diff --git a/flumine/strategy/strategy.py b/flumine/strategy/strategy.py
index 5f9d2f5..2cf4194 100644
--- a/flumine/strategy/strategy.py
+++ b/flumine/strategy/strategy.py
@@ -115,11 +115,17 @@ class BaseStrategy:
             del self._invested[i]
 
     # order
-    def place_order(self, market: Market, order) -> None:
+    def place_order(self, market: Market, order) -> bool:
+        """Returns True if passes
+        validate_order and placed.
+        """
         runner_context = self.get_runner_context(*order.lookup)
         if self.validate_order(runner_context, order):
             runner_context.place()
             market.place_order(order)
+            return True
+        else:
+            return False
 
     def cancel_order(self, market: Market, order, size_reduction: float = None) -> None:
         market.cancel_order(order, size_reduction)
diff --git a/flumine/worker.py b/flumine/worker.py
index 892d786..69143a9 100644
--- a/flumine/worker.py
+++ b/flumine/worker.py
@@ -33,7 +33,6 @@ class BackgroundWorker(threading.Thread):
         self.func_kwargs = func_kwargs if func_kwargs is not None else {}
         self.start_delay = start_delay
         self.context = context or {}
-        self._running = False
 
     def run(self) -> None:
         logger.info(
@@ -49,8 +48,7 @@ class BackgroundWorker(threading.Thread):
             },
         )
         time.sleep(self.start_delay)
-        self._running = True
-        while self._running:
+        while self.is_alive():
             logger.debug(
                 "BackgroundWorker {0} executing".format(self.name),
                 extra={
@@ -75,17 +73,6 @@ class BackgroundWorker(threading.Thread):
                 )
             time.sleep(self.interval)
 
-    def shutdown(self, timeout: int = 4) -> None:
-        logger.info(
-            "BackgroundWorker {0} shutting down".format(self.name),
-            extra={
-                "worker_name": self.name,
-                "function": self.function,
-            },
-        )
-        self._running = False
-        self.join(timeout)
-
 
 def keep_alive(context: dict, flumine) -> None:
     """Attempt keep alive if required or
diff --git a/requirements.txt b/requirements.txt
index 9a938fb..6c09bdd 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,4 @@
-betfairlightweight==2.9.2
+betfairlightweight==2.9.0
 tenacity==5.0.3
-python-json-logger==2.0.1
+python-json-logger==2.0.0
 requests


Test patch:
diff --git a/tests/test_clientcontrols.py b/tests/test_clientcontrols.py
index 75d967a..f795497 100644
--- a/tests/test_clientcontrols.py
+++ b/tests/test_clientcontrols.py
@@ -34,13 +34,13 @@ class TestBaseControl(unittest.TestCase):
         order = mock.Mock()
         order.info = {"hello": "world"}
         self.control._on_error(order, "test")
-        order.violation.assert_called_with()
+        order.violation.assert_called_with("Order has violated: None Error: test")
 
     def test_on_error_offset(self):
         order = mock.Mock()
         order.info = {"hello": "world"}
         self.control._on_error(order, "test")
-        order.violation.assert_called_with()
+        order.violation.assert_called_with("Order has violated: None Error: test")
 
 
 class TestMaxOrderCount(unittest.TestCase):
diff --git a/tests/test_order.py b/tests/test_order.py
index b103d5a..fc1390e 100644
--- a/tests/test_order.py
+++ b/tests/test_order.py
@@ -30,6 +30,7 @@ class BaseOrderTest(unittest.TestCase):
         self.assertIsNone(self.order.runner_status)
         self.assertIsNone(self.order.status)
         self.assertEqual(self.order.status_log, [])
+        self.assertIsNone(self.order.violation_msg)
         self.assertIsNone(self.order.bet_id)
         self.assertIsNone(self.order.EXCHANGE)
         self.assertEqual(self.order.update_data, {})
@@ -99,9 +100,10 @@ class BaseOrderTest(unittest.TestCase):
     @mock.patch("flumine.order.order.BaseOrder._update_status")
     def test_violation(self, mock__update_status):
         self.order.update_data = {123: 456}
-        self.order.violation()
+        self.order.violation("the murder capital")
         mock__update_status.assert_called_with(OrderStatus.VIOLATION)
         self.assertEqual(self.order.update_data, {})
+        self.assertEqual(self.order.violation_msg, "the murder capital")
 
     def test_place(self):
         with self.assertRaises(NotImplementedError):
diff --git a/tests/test_strategy.py b/tests/test_strategy.py
index ad21330..5496f62 100644
--- a/tests/test_strategy.py
+++ b/tests/test_strategy.py
@@ -150,10 +150,21 @@ class BaseStrategyTest(unittest.TestCase):
         mock_order = mock.Mock()
         mock_order.lookup = ("1", 2, 3)
         mock_market = mock.Mock()
-        self.strategy.place_order(mock_market, mock_order)
+        self.assertTrue(self.strategy.place_order(mock_market, mock_order))
         mock_market.place_order.assert_called_with(mock_order)
         self.assertIn(mock_order.lookup, self.strategy._invested)
 
+    @mock.patch(
+        "flumine.strategy.strategy.BaseStrategy.validate_order", return_value=False
+    )
+    def test_place_order_false(self, mock_validate_order):
+        mock_order = mock.Mock()
+        mock_order.lookup = ("1", 2, 3)
+        mock_market = mock.Mock()
+        self.assertFalse(self.strategy.place_order(mock_market, mock_order))
+        mock_market.place_order.assert_not_called()
+        self.assertIn(mock_order.lookup, self.strategy._invested)
+
     def test_cancel_order(self):
         mock_order = mock.Mock()
         mock_market = mock.Mock()
diff --git a/tests/test_worker.py b/tests/test_worker.py
index 31dace9..d0e42ba 100644
--- a/tests/test_worker.py
+++ b/tests/test_worker.py
@@ -13,34 +13,26 @@ class BackgroundWorkerTest(unittest.TestCase):
         self.worker = worker.BackgroundWorker(
             self.mock_flumine,
             self.mock_function,
-            0,
+            123,
             (1, 2),
             {"hello": "world"},
-            0,
+            5,
             {1: 2},
         )
 
     def test_init(self):
-        self.assertEqual(self.worker.interval, 0)
+        self.assertEqual(self.worker.interval, 123)
         self.assertEqual(self.worker.function, self.mock_function)
         self.assertEqual(self.worker.flumine, self.mock_flumine)
         self.assertEqual(self.worker.func_args, (1, 2))
         self.assertEqual(self.worker.func_kwargs, {"hello": "world"})
-        self.assertEqual(self.worker.start_delay, 0)
+        self.assertEqual(self.worker.start_delay, 5)
         self.assertEqual(self.worker.context, {1: 2})
         self.assertEqual(self.worker.name, "test")
-        self.assertFalse(self.worker._running)
 
     # def test_run(self):
     #     self.worker.run()
 
-    def test_shutdown(self):
-        self.worker.start()
-        self.assertTrue(self.worker.is_alive())
-        self.worker.shutdown()
-        self.assertFalse(self.worker._running)
-        self.assertFalse(self.worker.is_alive())
-
 
 class WorkersTest(unittest.TestCase):
     def setUp(self) -> None: