Better and more explicit Pydantic error messages

[tpvasconcelos]

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.

[achals]

Agreed @tpvasconcelos , we should definitely have better errors here. This should easy to do, we have a bunch of naked assert statements in feast/repo_config.py and just adding an error message there would hugely improve user experience.

Would you be willing to take this on?

[tpvasconcelos]

Automatically closed this by mistake. The merged PR was just an example implementation. I think more work needs to be done on this front.

@achals I don't have permission to reopen

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.