feat(catalog): add plugin infrastructure for extensible catalog types#2663
feat(catalog): add plugin infrastructure for extensible catalog types#2663Al-Pragliola wants to merge 1 commit intokubeflow:mainfrom
Conversation
Introduce catalog/pkg/plugin/ with the core building blocks for plugin-based catalog extensibility: CatalogPlugin interface, global registry with init()-based self-registration, and config loading that delegates to basecatalog's parser. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
pboyd
left a comment
pboyd
left a comment
There was a problem hiding this comment.
I had a few comments, but it looks good.
pkg normally implies an external interface, would we want to support external plugins? If not, maybe catalog/internal/plugin would be more appropriate.
| return result | ||
| } | ||
|
|
||
| // Get returns a plugin by name, or nil if not found. |
There was a problem hiding this comment.
| // Get returns a plugin by name, or nil if not found. | |
| // Get returns a plugin by name, or nil if not found. The returned boolean will be true if the plugin is found, and false otherwise. |
| for _, path := range paths { | ||
| cfg, err := LoadConfig(path) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Unless LoadConfig includes it, it would be helpful to include the file that failed here.
2 participants
Description
Introduces
catalog/pkg/plugin/— the core building blocks that allow different catalog types (models, MCP servers, datasets, etc.) to register themselves and be managed by a unified server.This package provides:
CatalogPlugininterface — standard contract for all catalog plugins: identity (Name,Version,Description), lifecycle (Init,Start,Stop,Healthy), HTTP routing (RegisterRoutes), and database migrations (Migrations).BasePathProvider(custom API base path),SourceKeyProvider(custom config key),FlagProvider(CLI flag registration).init()-based self-registration, deterministic iteration order, and duplicate detection (panics on conflict).basecatalog.ReadSourceConfig(), supporting single and multi-file loading.Why
catalog/pkg/plugin/instead ofpkg/catalog/plugin/The original plan placed the package at the repo root (
pkg/catalog/plugin/). During implementation we hit Go'sinternalpackage visibility rule: code underpkg/at the repo root cannot importcatalog/internal/...becauseinternalpackages are only visible to code rooted in their parent tree.This matters because the plugin config layer needs
basecatalog.SourceConfigandbasecatalog.ReadSourceConfig()fromcatalog/internal/catalog/basecatalog/. Rather than re-implementing ~200 lines of config parsing, YAML schema handling, and deprecated-field merging, we moved the package into thecatalog/module tree where the import is valid. This keeps a single source of truth for config parsing and ensures plugin tests exercise the exact same code paths as the production catalog server.What's NOT included (by design)
server.go) — plugin discovery, lifecycle management, and config routing are a follow-up ticket. This PR establishes the contract; the server wires it together.Config.SourceConfigcurrently carries*basecatalog.SourceConfigdirectly. Full decoupling (plugin-defined config types with raw YAML routing) is a server orchestration concern, documented in the code.How Has This Been Tested?
Merge criteria:
DCOcheck)ok-to-testhas been added to the PR.