kevinjqliu commented on code in PR #244: URL: https://github.com/apache/iceberg-go/pull/244#discussion_r1915921905
########## catalog/registry.go: ########## @@ -0,0 +1,135 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package catalog + +import ( + "fmt" + "maps" + "net/url" + "slices" + "strings" + "sync" + + "github.com/apache/iceberg-go" +) + +type registry map[string]Registrar + +func (r registry) getKeys() []string { + regMutex.Lock() + defer regMutex.Unlock() + return slices.Collect(maps.Keys(r)) +} + +func (r registry) set(catalogType string, reg Registrar) { + regMutex.Lock() + defer regMutex.Unlock() + r[catalogType] = reg +} + +func (r registry) get(catalogType string) (Registrar, bool) { + regMutex.Lock() + defer regMutex.Unlock() + reg, ok := r[catalogType] + return reg, ok +} + +func (r registry) remove(catalogType string) { + regMutex.Lock() + defer regMutex.Unlock() + delete(r, catalogType) +} + +var ( + regMutex sync.Mutex + defaultRegistry = registry{} +) + +// Registrar is a factory for creating Catalog instances, used for registering to use +// with LoadCatalog. +type Registrar interface { + GetCatalog(catalogURI string, props iceberg.Properties) (Catalog, error) +} + +type RegistrarFunc func(string, iceberg.Properties) (Catalog, error) + +func (f RegistrarFunc) GetCatalog(catalogURI string, props iceberg.Properties) (Catalog, error) { + return f(catalogURI, props) +} + +// Register adds the new catalog type to the registry. If the catalog type is already registered, it will be replaced. +func Register(catalogType string, reg Registrar) { + if reg == nil { + panic("catalog: RegisterCatalog catalog factory is nil") + } + defaultRegistry.set(catalogType, reg) +} + +// Unregister removes the requested catalog factory from the registry. +func Unregister(catalogType string) { + defaultRegistry.remove(catalogType) +} + +// GetRegsisteredCatalogs returns the list of registered catalog names that can +// be looked up via LoadCatalog. +func GetRegisteredCatalogs() []string { + return defaultRegistry.getKeys() +} + +// Load allows loading a specific catalog by URI and properties. +// +// This is utilized alongside RegisterCatalog/UnregisterCatalog to not only allow +// easier catalog loading but also to allow for custom catalog implementations to +// be registered and loaded external to this module. +// +// The URI is used to determine the catalog type by first checking if it contains +// the string "://" indicating the presence of a scheme. If so, the scheme is used +// to lookup the registered catalog. i.e. "glue://..." would return the Glue catalog +// implementation, passing the URI and properties to NewGlueCatalog. If no scheme is +// present, then the URI is used as-is to lookup the catalog factory function. Review Comment: Yea, for pyiceberg's `load_catalog`, [`name` is used to get predefined configs](https://github.com/apache/iceberg-python/blob/61b3510ded32270418ad54f5204113000d3dd07f/pyiceberg/catalog/__init__.py#L220-L224) from either the configuration file or env var. Otherwise, we [fallback to the propertie's `type` key](https://github.com/apache/iceberg-python/blob/61b3510ded32270418ad54f5204113000d3dd07f/pyiceberg/catalog/__init__.py#L227-L244), to see what type to load. If that doesnt exist, we [fallback to propertie's `uri`](https://github.com/apache/iceberg-python/blob/61b3510ded32270418ad54f5204113000d3dd07f/pyiceberg/catalog/__init__.py#L245-L246) as a proxy and try to infer the catalog type from that. The difference here is just which variable to use to infer the calalog type. The current PR uses `name` as a proxy whereas pyiceberg uses the propertie's `uri` field. Like you mention above, we can use `name` to fetch predefined configs from config file/env var and use propertie's `uri` as proxy to infer the catalog type. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
