-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Is your feature request related to a problem? Please describe.
The Pydantic errors can be very annoying to debug when the error messages are not explicit or (as often seen) completely missing. I provide two specific examples bellow but many more can be found throughout the codebase.
Describe the solution you'd like
Always include explicit error messages to all Pydating validation checks.
Additional context
A couple of examples:
>>> from feast import RepoConfig
>>> repo_config = RepoConfig(
... project="foo",
... )
Traceback (most recent call last):
File "my_script.py", line 2, in <module>
repo_config = RepoConfig(
File ".venv/lib/python3.9/site-packages/feast/repo_config.py", line 124, in __init__
super().__init__(**data)
File "pydantic/main.py", line 331, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 1 validation error for RepoConfig
__root__
(type=assertion_error)
👆 In this example, the feast.errors.FeastProviderNotSetError
exception should be raised instead of a blank AssertionError. The error message here would be explicit (i.e. "Provider is not set, but is required"
)
>>> from feast import RepoConfig
>>> repo_config = RepoConfig(
... project="foo",
... provider="local",
... offline_store=dict(
... type="my.custom.offline_store.CustomStore"
... ),
... )
Traceback (most recent call last):
File "my_script.py", line 2, in <module>
repo_config = RepoConfig(
File ".venv/lib/python3.9/site-packages/feast/repo_config.py", line 124, in __init__
super().__init__(**data)
File "pydantic/main.py", line 331, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 1 validation error for RepoConfig
__root__
(type=assertion_error)
👆 Just like in the other example, it is impossible to see what the issue here is. In this case, the get_offline_config_from_type
function should be raising an explicit exception with a "Offline store types should end with 'OfflineStore', got {offline_store_type} instead."
message instead of an empty AssertionError. This way it would be immediately clear what the issue was.