fix: auto-detect GCS/S3 registry store when registry is passed as string#6260
Open
jiwidi wants to merge 1 commit intofeast-dev:masterfrom
Open
fix: auto-detect GCS/S3 registry store when registry is passed as string#6260jiwidi wants to merge 1 commit intofeast-dev:masterfrom
jiwidi wants to merge 1 commit intofeast-dev:masterfrom
Conversation
a7ea9ab to
e8f6140
Compare
When `registry_config` is a string with a cloud URI scheme (gs://, s3://),
`RepoConfig.registry` previously hardcoded `get_registry_config_from_type("file")`,
producing a `RegistryConfig` that routes to `FileRegistryStore`. Since
`pathlib.Path` cannot handle cloud URIs (`Path("gs://...").is_absolute()` is
`False`), this caused `IsADirectoryError` at runtime.
Now the string branch creates a plain `RegistryConfig(path=...)` instead,
leaving `registry_store_type=None` so `Registry.__init__` can auto-detect
the correct store class via `get_registry_store_class_from_scheme()`.
Signed-off-by: jiwidi <fhjaime96@gmail.com>
e8f6140 to
4ee48c2
Compare
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
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.
Problem
When passing
registry="gs://bucket/feast/registry.pb"as a string toRepoConfig, Feast always creates a file-basedRegistryConfig(hardcoded atrepo_config.pyline ~402). This ignores the URI scheme and causesFileRegistryStoreto be used, which cannot handlegs://paths —pathlib.Path("gs://...")is not treated as absolute, leading toIsADirectoryErrorat runtime.The
RegistryConfigdocstring states:But
FileRegistryStoreusespathlib.Pathwhich doesn't support GCS URIs.Meanwhile,
Registry.__init__already has correct scheme-based auto-detection viaget_registry_store_class_from_scheme()andREGISTRY_STORE_CLASS_FOR_SCHEME = {"gs": "GCSRegistryStore", ...}— it just never gets a chance because theRegistryConfigis pre-wrapped as "file" type.Root Cause
In
RepoConfig.registryproperty, theelif isinstance(self.registry_config, str)branch hardcodes:This wraps the path in a file-type config regardless of the URI scheme.
Reproduction
Fix
In
RepoConfig.registryproperty, when registry is a string, create a plainRegistryConfig(path=...)instead of hardcodingget_registry_config_from_type("file"). This preservesregistry_store_type=None, letting the existing auto-detection inRegistry.__init__select the correct store class based on the URI scheme.Local file paths continue to work because
registry_typedefaults to"file"inRegistryConfig, andget_registry_store_class_from_schememaps thefilescheme (and schemeless paths) toFileRegistryStore.Tests
Added
test_registry_string_config.pywith 11 tests covering:RegistryConfigwithregistry_store_type=NoneREGISTRY_STORE_CLASS_FOR_SCHEMEmaps gs→GCS, s3→S3, file→Filepathlib.Pathcannot handle cloud URIs (documenting the underlying issue)