Skip to content

refactor!: cleanup namespace related APIs#6186

Open
jackye1995 wants to merge 7 commits intolance-format:mainfrom
jackye1995:latest-so
Open

refactor!: cleanup namespace related APIs#6186
jackye1995 wants to merge 7 commits intolance-format:mainfrom
jackye1995:latest-so

Conversation

@jackye1995
Copy link
Contributor

  1. deprecate storage_options_provider in python and java (will be removed in 5.0.0), because to make managed verisoning work, we have updated the codebase to pass namespace and table ID into the python and java binding layer. It becomes unnecessary for us to do a language specific storage_options_provider and then bind that to rust, because we can directly construct the rust StorageOptionsProvider using binded namespace client.
  2. rename all namespace to namespace_client. This is done for all new code added in 1. For rust we have to hard-rename the variables. For python and java we keep the same rule for deprecation and removal in 5.0.0. This rename is based on community feedback, and aims at clarifying the concept of Namespace Client, which is a client of a namespace implementation for a Namespace SDK of specific language, not a namespace path like ["ns1", "ns2"].

@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2026

PR Review: refactor!: cleanup namespace related APIs

Overall this is a clean rename + deprecation PR. A few issues worth addressing:

P1: Double deprecation warnings for storage_options_provider

The storage_options_provider deprecation warning is emitted both in dataset() (in __init__.py) and in LanceDataset.__init__(). When calling dataset(storage_options_provider=...), the user will see the same warning twice. Consider only warning in the public entry point (dataset()) and not in LanceDataset.__init__, or gate the inner warning.

P1: Minor doc inconsistency in write_fragments

In fragment.py, the deprecation notice for storage_options_provider in write_fragments says "When using namespace and table_id" — should be namespace_client and table_id to match the new API.


🤖 Generated with Claude Code

@codecov
Copy link

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 52.00000% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-io/src/object_store/storage_options.rs 0.00% 9 Missing ⚠️
rust/lance/src/dataset.rs 71.42% 2 Missing ⚠️
rust/lance/src/dataset/builder.rs 85.71% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

table_id: Optional[List[str]] = None,
storage_options_provider: Optional[Any] = None,
# Deprecated parameters
namespace: Optional[LanceNamespace] = None,
Copy link
Member

@westonpace westonpace Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Humorous observation: technically moving arguments without a * in the argument list is a breaking change but if anyone was using this argument as a positional argument at this point then I worry

Comment on lines 102 to +103
storage_options_provider: Optional[Any] = None,
# Deprecated parameters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
storage_options_provider: Optional[Any] = None,
# Deprecated parameters
# Deprecated parameters
storage_options_provider: Optional[Any] = None,

@@ -431,14 +431,38 @@ def __init__(
read_params: Optional[Dict[str, Any]] = None,
session: Optional[Session] = None,
storage_options_provider: Optional[Any] = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to deprecated parameters section?

Comment on lines +3709 to +3717
if storage_options_provider is not None:
warnings.warn(
"The 'storage_options_provider' parameter is deprecated and will be "
"removed in version 5.0.0. When using 'namespace_client' and "
"'table_id', the storage options provider is created automatically. "
"Pass 'storage_options' instead for static or initial storage options.",
DeprecationWarning,
stacklevel=2,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warning is repeated all over the place. In a pre-AI world I'd probably say we should make a helper method. Now I'll just say it might be a good idea.


**Only valid in CREATE mode**. Will raise an error if used with
APPEND/OVERWRITE modes.
namespace_client : optional, LanceNamespace
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we document these things in the site docs anywhere? I don't see any changes in this PR to those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants