Repository navigation
fix: coerce dict-style requests into DescribeTableRequest in catalog impls - #182
Open
ZackFairTS wants to merge 1 commit into
Open
ZackFairTS wants to merge 1 commit into
ZackFairTS wants to merge 1 commit into
Conversation
…impls pylance's Rust namespace bridge (python/src/namespace.rs) invokes the Python namespace impl's describe_table with a DictWithModelDump (a plain dict subclass exposing only model_dump(), no attributes) during the write-path storage-options callback. The catalog impls (glue, hive2, hive3, unity, iceberg, polaris) access request fields as attributes (request.load_detailed_metadata), which raises AttributeError. pylance's own DirectoryNamespace/RestNamespace avoid this by calling request.model_dump(). Coerce a non-model request into the real DescribeTableRequest via from_dict at the top of each describe_table. Adds a regression test per impl feeding a DictWithModelDump. Fixes lance-format#181
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
pylance's Rust namespace bridge (python/src/namespace.rs) invokes the Python namespace implementation'sdescribe_tablewith aDictWithModelDump— a plaindictsubclass that exposes onlymodel_dump()and no attributes — during the write-path storage-options callback (lance-iofetch_storage_options).The catalog impls in this package access request fields as attributes (
request.load_detailed_metadata, thenrequest.id, ...), so they raise:which surfaces end-to-end as:
pylance's own
DirectoryNamespace/RestNamespacewrappers avoid this because they callrequest.model_dump(); the catalog impls here use attribute access and therefore break on the write path.Changes
DescribeTableRequest(request = DescribeTableRequest.from_dict(dict(request))) at the top ofdescribe_tablein all six catalog impls:glue,hive2,hive3,unity,iceberg,polaris. The check is a no-op when a real model is already passed, so existing callers are unaffected.test_describe_table_accepts_dict_request) that feeds aDictWithModelDumpand assertsdescribe_tablesucceeds (noAttributeError) and returns the correct location.describe_tableis the method exercised by the pylance Rust storage-options write callback, so it is the minimum required fix. Other request-taking methods share the same attribute-access pattern and could be coerced similarly in a follow-up if desired.Verification
cd python && uv sync --extra glue --extra hive2 --extra hive3 --extra unity --extra iceberg --extra polaris --extra devruff check+ruff format --checkon all six impls and their tests: clean.pytest tests/test_{glue,hive2,hive3,unity,iceberg,polaris}.py: 129 passed (incl. the 6 new tests).Fixes #181