feat: add catalog namespace support and refactor adapter implementation#14
feat: add catalog namespace support and refactor adapter implementation#14kaori-seasons wants to merge 3 commits intolance-format:mainfrom
Conversation
0b4ded7 to
dca8979
Compare
This commit introduces a new namespace abstraction layer for Lance catalog integration with Flink. Key components added: - AbstractLanceNamespaceAdapter: Interface defining namespace operations - LanceNamespaceAdapter: Implementation with direct Lance Namespace SDK API calls - LanceNamespaceConfig: Type-safe configuration management with ImplType enum - BaseLanceNamespaceCatalog: Base catalog implementation for Flink integration - LanceNamespaceAdapterITCase: Comprehensive integration tests (23 test cases) - MockLanceNamespace: Mock implementation for standalone testing Features: - Direct API calls to Lance Namespace SDK (no reflection) - Support for both directory and REST namespace implementations - Complete CRUD operations for namespaces and tables - Full metadata management - Production-ready error handling and resource management
dca8979 to
a97baa2
Compare
| public static final String KEY_IMPL = "impl"; | ||
| public static final String KEY_ROOT = "root"; | ||
| public static final String KEY_URI = "uri"; | ||
| public static final String KEY_EXTRA_LEVEL = "extra_level"; |
There was a problem hiding this comment.
feels like it is time that we add something like 3LevelLanceNamespace in https://github.com/lance-format/lance-namespace/pulls so that we can just have a consistent experience across the engines instead of implementing exactly the same thing here. Would you be up to that work?
There was a problem hiding this comment.
Thank you for your reply. Due to the large volume of emails, I only just saw your message. Please allow me some time to review the relevant code.
| * This mock allows tests to run without requiring the actual Lance Namespace | ||
| * library to be available. It simulates the basic behavior of the real API. | ||
| */ | ||
| public class MockLanceNamespace { |
There was a problem hiding this comment.
we should always default to test with DirectoryNamespace and RestNamespace with a DirectoryNamespace backend, since those 2 come out of box with lance-core and DirectoryNamespace is storage only.
| * - Error handling: duplicate creation, non-existing resources and other exception scenarios | ||
| */ | ||
| @DisplayName("Lance Namespace Adapter Integration Test") | ||
| class LanceNamespaceAdapterITCase { |
There was a problem hiding this comment.
this is just calling the adapter. It's not really exercising interaction with Flink through operations like CREATE CATALOG, SHOW DATABASES, SHOW TABLES
- Add BaseLanceNamespaceCatalog with full Flink Catalog API support - Add LanceCatalogFactory for creating namespace adapters - Refactor LanceNamespaceAdapter with improved CRUD operations - Update LanceNamespaceConfig with extra_level and parent support
…e backend - Remove MockLanceNamespace as lance-core natively supports DirectoryNamespace and RestNamespace - Update LanceNamespaceAdapterITCase to use real DirectoryNamespace backend for testing - Add nested RestNamespaceTests enabled via LANCE_REST_URI environment variable - DirectoryNamespace is used for storage tests, RestNamespace for API tests when available
| /** | ||
| * Adapter for Lance Namespace API. | ||
| * | ||
| * Provides unified interface for interacting with Lance Namespace, |
There was a problem hiding this comment.
Lance Namespace is supposed to be the unified interface to implement Flink AbstractCatalog, I don't think you need to wrap it in yet another layer of adapter
Related to issue-2