burr icon indicating copy to clipboard operation
burr copied to clipboard

fix: Improve error message for `SQLitePersister`

Open zilto opened this issue 1 year ago • 4 comments

I always forget about calling .initialize() on the persister to create tables and that's the error I get:

  File .../assistant.py", line 86, in build_assistant
    .build()
     ^^^^^^^
  File ".../burr/telemetry.py", line 276, in wrapped_fn
    return call_fn(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../application.py", line 2333, in build
    self._load_from_persister()
  File ".../burr/core/application.py", line 2262, in _load_from_persister
    load_result = self.initializer.load(_partition_key, _app_id, _sequence_id)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../burr/core/persistence.py", line 242, in load
    cursor.execute(
sqlite3.OperationalError: no such table: burr_state

Notes:

  • it's a bit odd that the stack trace bounces between user code -> burr.telemetry -> user code -> burr.core.persistence
  • could catch the uninitialized persister at ApplicationBuilder().with_state_persistence(...) instead of ApplicationBuilder().build(). (seems like a unlikely pattern to pass the persister then initialize it later)
  • error message should say RuntimeError: Uninitialized persister. Make sure to call .initialize() before passing it to the ApplicationBuilder

zilto avatar Nov 06 '24 15:11 zilto

@zilto can I take it up?

ghost avatar Nov 08 '24 06:11 ghost

@arpitgupta-it Yes, you can work on it! Do you have any question? The goal is to catch more explicitly the missing table when calling self.initializer.load(...) and raise an appropriate error message

zilto avatar Nov 08 '24 18:11 zilto

I just raised a PR. Please review if it looks good to approve or any changes.

ghost avatar Nov 08 '24 18:11 ghost

@arpitgupta-it thanks I just added comments. We need to extend it to cover all persisters and maintain backwards compatibility with custom persisters people may have written.

skrawcz avatar Nov 08 '24 22:11 skrawcz