Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions marimo/_sql/engines/adbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
InferenceConfig,
SQLConnection,
default_inference_config,
fabricates_attributes,
)
from marimo._sql.utils import convert_to_output, is_cheap_dialect
from marimo._types.ids import VariableName
Expand Down Expand Up @@ -461,6 +462,11 @@ def is_compatible(var: Any) -> bool:
if var_type_name == "ibis.common.deferred.Deferred":
return False

# Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics)
# pass any getattr-based check and hang catalog introspection. #10213
if fabricates_attributes(var):
return False

try:
# First, validate the connection-level surface area to avoid
# accidentally classifying regular DB-API connections as ADBC.
Expand Down
7 changes: 6 additions & 1 deletion marimo/_sql/engines/dbapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import TYPE_CHECKING, Any, Protocol

from marimo import _loggers
from marimo._sql.engines.types import QueryEngine
from marimo._sql.engines.types import QueryEngine, fabricates_attributes
from marimo._sql.utils import convert_to_output

LOGGER = _loggers.marimo_logger()
Expand Down Expand Up @@ -112,6 +112,11 @@ def is_compatible(var: Any) -> bool:
if var_type_name == "ibis.common.deferred.Deferred":
return False

# Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics)
# pass any getattr-based check and hang catalog introspection. #10213
if fabricates_attributes(var):
return False

try:
required_methods = ["cursor", "commit", "rollback", "close"]
has_required_methods = all(
Expand Down
22 changes: 22 additions & 0 deletions marimo/_sql/engines/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,28 @@

NO_SCHEMA_NAME = ""

# Attribute that no real database connection defines; used to detect objects
# that fabricate attributes via a permissive __getattr__.
_MISSING_ATTRIBUTE_PROBE = "_marimo_does_not_exist_"


def fabricates_attributes(var: Any) -> bool:
"""Whether `var` fabricates attributes via a permissive `__getattr__`.

Some objects return a new object for *any* attribute name (for example
pytorch-ignite metrics, which build lazy `MetricsLambda` graphs). Such
objects pass every `getattr`-based duck-typing check, and catalog
introspection on them can hang. Genuine connection proxies that forward
`__getattr__` to an underlying connection are unaffected, since the
probe attribute does not exist there either.
"""
try:
return getattr(var, _MISSING_ATTRIBUTE_PROBE, None) is not None
except Exception:
# A __getattr__ that raises something other than AttributeError is
# not a usable connection either.
return True


@dataclass
class InferenceConfig(ABC):
Expand Down
14 changes: 14 additions & 0 deletions tests/_sql/test_adbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,20 @@ def test_adbc_execute_native_returns_arrow_table(monkeypatch) -> None:
assert cursor.did_close is True


def test_adbc_is_compatible_rejects_fabricated_attributes() -> None:
# Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics)
# resolve any attribute name, so they pass getattr-based duck typing
# and catalog introspection on them hangs. See #10213.
class FabricatingAttributes:
def __getattr__(self, name: str) -> object:
def _lazy(*_args: object, **_kwargs: object) -> object:
return FabricatingAttributes()

return _lazy

assert AdbcDBAPIEngine.is_compatible(FabricatingAttributes()) is False


def test_adbc_is_compatible_does_not_create_cursor() -> None:
conn = FakeAdbcDbApiConnection(
cursor=FakeAdbcDbApiCursor(description=None),
Expand Down
14 changes: 14 additions & 0 deletions tests/_sql/test_dbapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ def test_is_compatible() -> None:
assert not isinstance(engine, EngineCatalog)


def test_is_compatible_rejects_fabricated_attributes() -> None:
# Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics)
# resolve any attribute name, so they pass getattr-based duck typing
# and catalog introspection on them hangs. See #10213.
class FabricatingAttributes:
def __getattr__(self, name: str) -> object:
def _lazy(*_args: object, **_kwargs: object) -> object:
return FabricatingAttributes()

return _lazy

assert not DBAPIEngine.is_compatible(FabricatingAttributes())


def test_execute_native(dbapi_engine: DBAPIEngine) -> None:
with patch.object(
dbapi_engine, "sql_output_format", return_value="native"
Expand Down
16 changes: 16 additions & 0 deletions tests/_sql/test_get_engines.py
Original file line number Diff line number Diff line change
Expand Up @@ -482,3 +482,19 @@ def test_variables_without_datasource_engine() -> None:
variables = [("deferred_for_test", deferred_for_test)]
engines = get_engines_from_variables(variables)
assert not engines


def test_fabricated_attributes_not_treated_as_engine() -> None:
# Objects with a permissive __getattr__ (e.g. pytorch-ignite metrics)
# resolve any attribute name, so they pass getattr-based duck typing
# and catalog introspection on them hangs. See #10213.
class FabricatingAttributes:
def __getattr__(self, name: str) -> object:
def _lazy(*_args: object, **_kwargs: object) -> object:
return FabricatingAttributes()

return _lazy

variables = [(VariableName("metric"), FabricatingAttributes())]
engines = get_engines_from_variables(variables)
assert not engines
Loading