Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data engine _create_new_book failure upon missing instrument #2237

Closed
ryantam626 opened this issue Jan 23, 2025 · 2 comments
Closed

Data engine _create_new_book failure upon missing instrument #2237

ryantam626 opened this issue Jan 23, 2025 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@ryantam626
Copy link
Contributor

Bug Report

Expected Behavior

Creating new book for non-existent instrument should not fail.

Actual Behavior

It fails.

Steps to Reproduce the Problem

Omitted.

Details

if instrument is None:
self._log.error(
f"Cannot subscribe to {instrument.id} <OrderBook> data: "
f"no instrument found in the cache",
)
return

Attempting to use instrument.id when we know it's None.

Specifications

  • OS platform: WSL2
  • Python version: 3.11.10
  • nautilus_trader version: latest develop SHA fafcc30 debug build
@ryantam626 ryantam626 added the bug Something isn't working label Jan 23, 2025
@ryantam626
Copy link
Contributor Author

ryantam626 commented Jan 23, 2025

Works but it's fairly disgusting to pass both ID and instrument itself 😓

From 5dd6ea386776d56731fa125433a1fb0f3d6884e5 Mon Sep 17 00:00:00 2001
From: Ryan Tam <[email protected]>
Date: Thu, 23 Jan 2025 20:46:37 +0000
Subject: [PATCH] fix: Don't blow up when creating book for non-existent
 instrument

---
 nautilus_trader/data/engine.pxd | 2 +-
 nautilus_trader/data/engine.pyx | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/nautilus_trader/data/engine.pxd b/nautilus_trader/data/engine.pxd
index 7c1ccd6e0..e32e7a668 100644
--- a/nautilus_trader/data/engine.pxd
+++ b/nautilus_trader/data/engine.pxd
@@ -135,7 +135,7 @@ cdef class DataEngine(Component):
     cpdef void _handle_subscribe_order_book_deltas(self, MarketDataClient client, InstrumentId instrument_id, BookType book_type, uint64_t depth, bint managed, dict params)  # noqa
     cpdef void _handle_subscribe_order_book(self, MarketDataClient client, InstrumentId instrument_id, BookType book_type, uint64_t depth, uint64_t interval_ms, bint managed, dict params)  # noqa
     cpdef void _setup_order_book(self, MarketDataClient client, InstrumentId instrument_id, BookType book_type, uint64_t depth, bint only_deltas, bint managed, dict params)  # noqa
-    cpdef void _create_new_book(self, Instrument instrument, BookType book_type)
+    cpdef void _create_new_book(self, InstrumentId instrument_id, Instrument instrument, BookType book_type)
     cpdef void _handle_subscribe_quote_ticks(self, MarketDataClient client, InstrumentId instrument_id, dict params)
     cpdef void _handle_subscribe_synthetic_quote_ticks(self, InstrumentId instrument_id)
     cpdef void _handle_subscribe_trade_ticks(self, MarketDataClient client, InstrumentId instrument_id, dict params)
diff --git a/nautilus_trader/data/engine.pyx b/nautilus_trader/data/engine.pyx
index 1fc59ffd4..57fbe366c 100644
--- a/nautilus_trader/data/engine.pyx
+++ b/nautilus_trader/data/engine.pyx
@@ -909,10 +909,10 @@ cdef class DataEngine(Component):
                 root = instrument_id.symbol.root()
                 instruments = self._cache.instruments(venue=instrument_id.venue, underlying=root)
                 for instrument in instruments:
-                    self._create_new_book(instrument, book_type)
+                    self._create_new_book(instrument_id, instrument, book_type)
             else:
                 instrument = self._cache.instrument(instrument_id)
-                self._create_new_book(instrument, book_type)
+                self._create_new_book(instrument_id, instrument, book_type)
 
         # Always re-subscribe to override previous settings
         try:
@@ -959,10 +959,10 @@ cdef class DataEngine(Component):
                 priority=10,
             )
 
-    cpdef void _create_new_book(self, Instrument instrument, BookType book_type):
+    cpdef void _create_new_book(self, InstrumentId instrument_id, Instrument instrument, BookType book_type):
         if instrument is None:
             self._log.error(
-                f"Cannot subscribe to {instrument.id} <OrderBook> data: "
+                f"Cannot subscribe to {instrument_id} <OrderBook> data: "
                 f"no instrument found in the cache",
             )
             return
-- 
2.43.0

@cjdsellers cjdsellers self-assigned this Jan 23, 2025
@cjdsellers cjdsellers added enhancement New feature or request and removed bug Something isn't working labels Jan 23, 2025
@cjdsellers
Copy link
Member

Hey @ryantam626

Thanks for the report.

I just removed the bug label, because the system is operating within spec here (error is intentional).

Investigating the code though, I think we can relax this to just log a warning for now, as only the instrument_id is actually needed for the book setup. I can understand the use case where you may want to subscribe and have a book ready - but the instrument is not yet in the cache for any reason (per #2238).

For backtests, we already validate that instruments are in the cache for all data, so this change will just make things more flexible for different live workflows/scenarios.

Now improved with 38b7138.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

2 participants