Skip to content

feat: add catalog namespace support and refactor adapter implementation#14

Open
kaori-seasons wants to merge 3 commits intolance-format:mainfrom
kaori-seasons:support-catalog
Open

feat: add catalog namespace support and refactor adapter implementation#14
kaori-seasons wants to merge 3 commits intolance-format:mainfrom
kaori-seasons:support-catalog

Conversation

@kaori-seasons
Copy link
Copy Markdown

Related to issue-2

  • Add namespace abstraction layer with AbstractLanceNamespaceAdapter
  • Implement LanceNamespaceAdapter for direct Lance Namespace SDK API calls
  • Add LanceNamespaceConfig for type-safe configuration management
  • Implement BaseLanceNamespaceCatalog for Flink catalog integration
  • Add comprehensive integration tests (LanceNamespaceAdapterITCase)
  • Add MockLanceNamespace for standalone testing
  • Remove reflection-based calls in favor of direct API invocations
  • Eliminate hardcoded strings with ImplType enum
  • Update all related catalog implementations

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
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";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I've made some changes.
348cd43

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants