Feat/catalog type selector#2633
Draft
Kurtiscwright wants to merge 2 commits into
Draft
Conversation
Add a `CatalogType` enum to the catalog loader that lets callers select a built-in catalog by a declared variant (`CatalogType::Rest`) instead of a bare type string, with an inherent `async fn load` that dispatches via an exhaustive match. Including `Memory` closes a gap where the in-memory catalog was absent from the string registry. This is purely additive: the existing free `load(&str)`, `supported_types()`, `CatalogLoader`, and `BoxedCatalogBuilder` are left unchanged and continue to work alongside the new selector. This approach will be repeated for how file format's registry will be implemented. Applying the approach here is aimed at improving the Catalog selection to be more idiomatic and to get alignment from the community on how we want to structure registries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR was written with help from LLM based tools.
Which issue does this PR close?
What changes are included in this PR?
Add a
CatalogTypeenum to the catalog loader that lets callersselect a built-in catalog by a declared variant
(
CatalogType::Rest) instead of a bare type string,with an inherent
async fn loadthat dispatches via anexhaustive match.
Including
Memorycloses a gap where the in-memory catalog wasabsent from the string registry.
This is purely additive: the existing free
load(&str),supported_types(),CatalogLoader, andBoxedCatalogBuilderareleft unchanged and continue to work alongside the new selector.
This approach will be repeated for how file format's registry will be
implemented. Applying the approach here is aimed at improving the
Catalog selection to be more idiomatic and to get alignment from the
community on how we want to structure registries.
Are these changes tested?
Brand new unit tests to validate that storage factory is passed through correctly as well as overall functionality is used.