refactor!: cleanup namespace related APIs#6186
refactor!: cleanup namespace related APIs#6186jackye1995 wants to merge 7 commits intolance-format:mainfrom
Conversation
PR Review: refactor!: cleanup namespace related APIsOverall this is a clean rename + deprecation PR. A few issues worth addressing: P1: Double deprecation warnings for
|
Codecov Report❌ Patch coverage is 📢 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, |
There was a problem hiding this comment.
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
| storage_options_provider: Optional[Any] = None, | ||
| # Deprecated parameters |
There was a problem hiding this comment.
| 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, | |||
There was a problem hiding this comment.
Move this to deprecated parameters section?
| 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, | ||
| ) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Do we document these things in the site docs anywhere? I don't see any changes in this PR to those.
storage_options_providerin 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 specificstorage_options_providerand then bind that to rust, because we can directly construct the rustStorageOptionsProviderusing binded namespace client.namespacetonamespace_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"].