Skip to content

Commit 72c4373

Browse files
kurodo3[bot]claude
authored andcommitted
fix(sources): avoid calling closed SpiralDBConnector in to_config
SpiralDBTableSource.to_config() was delegating to super().to_config() (DBTableSource), which called self._connector.to_config(). Because SpiralDBConnector.to_config() raises RuntimeError once closed, and the connector is always closed in __init__ after the eager data load, every call to to_config() on a real SpiralDBTableSource instance would raise. Fix: override to_config() to build the dict directly from the already- captured instance attributes, bypassing the closed connector entirely. Also update _make_mock_connector() in the test suite so close() flips a flag and to_config() raises RuntimeError when closed, matching real SpiralDBConnector behaviour. Add test_to_config_works_after_connector_is_closed as an explicit regression guard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d692bee commit 72c4373

2 files changed

Lines changed: 62 additions & 5 deletions

File tree

src/orcapod/core/sources/spiraldb_table_source.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,26 @@ def __init__(
138138
pass
139139

140140
def to_config(self) -> dict[str, Any]:
141-
"""Serialize source configuration to a JSON-compatible dict."""
142-
base = super().to_config()
143-
base.pop("connector", None)
141+
"""Serialize source configuration to a JSON-compatible dict.
142+
143+
Note:
144+
Does **not** call ``super().to_config()`` (i.e. ``DBTableSource.to_config()``)
145+
because that would invoke ``self._connector.to_config()``, which raises
146+
``RuntimeError`` once the connector has been closed. The connector is always
147+
closed during ``__init__`` after the eager data load, so the full dict is
148+
built from the already-captured instance attributes instead.
149+
"""
144150
return {
145-
**base,
146151
"source_type": "spiraldb_table",
147152
"project_id": self._project_id,
148153
"dataset": self._dataset,
149154
"overrides": self._overrides,
155+
"table_name": self._table_name,
156+
"tag_columns": list(self._tag_columns),
157+
"system_tag_columns": list(self._system_tag_columns),
158+
"record_id_column": self._record_id_column,
159+
"source_id": self.source_id,
160+
**self._identity_config(),
150161
}
151162

152163
@classmethod

tests/test_core/sources/test_spiraldb_table_source.py

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,12 @@ def _make_mock_connector(
3838
pk_columns: list[str] | None = None,
3939
batches: list[pa.RecordBatch] | None = None,
4040
) -> MagicMock:
41-
"""Return a MagicMock that satisfies DBConnectorProtocol for the given data.
41+
"""Return a MagicMock that faithfully simulates SpiralDBConnector lifecycle.
42+
43+
Specifically, ``close()`` flips a closed flag and ``to_config()`` raises
44+
``RuntimeError`` once closed — matching the real ``SpiralDBConnector``
45+
behaviour. This ensures tests catch bugs where ``to_config()`` (or any
46+
other post-close call) is accidentally delegated to the connector.
4247
4348
Args:
4449
table_names: Tables reported by ``get_table_names()``.
@@ -63,6 +68,27 @@ def _make_mock_connector(
6368
connector.get_table_names.return_value = table_names
6469
connector.get_pk_columns.return_value = pk_columns
6570
connector.iter_batches.return_value = iter(batches)
71+
72+
# Simulate the closed-connector lifecycle: close() flips the flag and
73+
# to_config() raises RuntimeError afterwards, exactly as the real
74+
# SpiralDBConnector does.
75+
connector._closed = False
76+
77+
def _close():
78+
connector._closed = True
79+
80+
def _to_config():
81+
if connector._closed:
82+
raise RuntimeError("SpiralDBConnector is closed")
83+
return {
84+
"connector_type": "spiraldb",
85+
"project_id": _PROJECT_ID,
86+
"dataset": _DATASET,
87+
"overrides": None,
88+
}
89+
90+
connector.close.side_effect = _close
91+
connector.to_config.side_effect = _to_config
6692
return connector
6793

6894

@@ -607,6 +633,26 @@ def test_to_config_has_identity_fields(self):
607633
assert "content_hash" in cfg
608634
assert "pipeline_hash" in cfg
609635

636+
def test_to_config_works_after_connector_is_closed(self):
637+
"""to_config() must not call the connector — it is closed after __init__.
638+
639+
Regression test: the mock connector's to_config() raises RuntimeError
640+
when closed (matching SpiralDBConnector's behaviour). If
641+
SpiralDBTableSource.to_config() delegates to the connector, this test
642+
will raise RuntimeError and fail.
643+
"""
644+
from orcapod.core.sources import SpiralDBTableSource
645+
646+
connector = _make_mock_connector()
647+
with _patch_connector(connector):
648+
src = SpiralDBTableSource(_PROJECT_ID, _TABLE_NAME)
649+
# Connector is closed at this point; calling to_config() must not
650+
# raise.
651+
assert connector._closed, "connector should be closed after __init__"
652+
cfg = src.to_config() # must not raise RuntimeError
653+
assert cfg["source_type"] == "spiraldb_table"
654+
assert cfg["project_id"] == _PROJECT_ID
655+
610656
def test_from_config_reconstructs_successfully(self):
611657
from orcapod.core.sources import SpiralDBTableSource
612658

0 commit comments

Comments
 (0)