SqlRegistryConfig does not exist

[connorwaldman]

Expected Behavior

When specifying "sql" as the registry type via the python SDK, feast uses the appropriate config class.

Current Behavior

When specifying "sql" as the registry type via the python SDK, calling the FeatureStore object results in this error:
feast.errors.FeastClassImportError: Could not import class 'SqlRegistryConfig' from module 'feast.infra.registry.sql'

get_registry_config_from_type builds the class name here: https://github.com/feast-dev/feast/blob/master/sdk/python/feast/repo_config.py#L499 causing the config to look for the SqlRegistryConfig class, which doesn't exist. This is due to the code adding Config to the end of the current registry class, which works when specifying "file" (which uses the RegistryConfig class, which exists), but not for "sql".

Steps to reproduce

Set my_repo_config = RepoConfig(registry=RepoConfig(registry_type="sql")) and then call FeatureStore(config=my_repo_config), which will then result in the error described above.

Specifications

• Version: 0.30.0
• Platform: Ubuntu
• Subsystem:

Possible Solution

Option 1:
Change the REGISTRY_CLASS_FOR_TYPE dict so that "sql" maps to "feast.infra.registry.registry.Registry" instead of "feast.infra.registry.sql.SqlRegistry" - This may also have to be done for "snowflake.registry"

Option 2:
Create the SqlRegistryConfig class in feast.infra.registry.sql. I'm also not seeing a SnowflakeRegistryConfig class in the feast.infra.registry.snowflake module, so that may need to be created as well.

Option 3:
Alter the get_registry_config_from_type function so it builds out the class names differently based on they registry type
https://github.com/feast-dev/feast/blob/master/sdk/python/feast/repo_config.py#L499

[GDegrove]

From what I can see, there's already to type of pattern for loading the configuration class for a type.

Most of the config uses the class name to derive the config, except the feature server that uses a specific ENUM because of the We do not support custom Feature Server
https://github.com/feast-dev/feast/blob/master/sdk/python/feast/repo_config.py#L535

Looking at this, I guess it's safe to say that we should create a SqlRegistryConfig class for the SQLRegistry class, to follow the other design pattern. At first, the SQLRegistryConfig would be as easy as being an extension of the Registry, then i required it can become more complex.

I see that this bug priority is p2. Does that mean that it will be picked up ASAP?

If you are ok with the plan, I think we can open a PR and propose a fix for this issue quite fast (it does seem to be a low-hanging fruit solution, that would work).
Maybe there's other concern that I don't grasp?

Edit: this still exists in version 0.30.2

[davidschuler-8451]

hi @GDegrove & @connorwaldman - I attempted a PR for this fix. Confirmed it is behaving as expected on my local tests. could you please take a look and see if this is what you had in mind? #3586

thanks!

[connorwaldman]

@davidschuler-8451 Yeah looks great! Thanks!