fix: More explicit error messages #2708

tpvasconcelos · 2022-05-16T20:33:51Z

What this PR does / why we need it:

This PR adds more explicit error messages to validation checks (for better Pydantic error messages).

tpvasconcelos · 2022-05-16T20:35:34Z

@achals here's an example for the repo_config.py module. If this is fine with you, I'll work on another PR later addressing more of these cases 👍

codecov-commenter · 2022-05-16T20:42:30Z

Codecov Report

Merging #2708 (5f6e645) into master (5d6fa94) will decrease coverage by 22.33%.
The diff coverage is 45.45%.

@@             Coverage Diff             @@
##           master    #2708       +/-   ##
===========================================
- Coverage   80.99%   58.65%   -22.34%     
===========================================
  Files         166      166               
  Lines       13890    13899        +9     
===========================================
- Hits        11250     8153     -3097     
- Misses       2640     5746     +3106

Flag	Coverage Δ
integrationtests	`?`
unittests	`58.65% <45.45%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
sdk/python/feast/repo_config.py	`78.41% <37.50%> (-10.38%)`	⬇️
sdk/python/feast/errors.py	`67.05% <66.66%> (-4.20%)`	⬇️
.../integration/online_store/test_online_retrieval.py	`16.84% <0.00%> (-83.16%)`	⬇️
sdk/python/tests/utils/online_read_write_test.py	`19.35% <0.00%> (-80.65%)`	⬇️
.../integration/online_store/test_universal_online.py	`17.64% <0.00%> (-77.78%)`	⬇️
...fline_store/test_universal_historical_retrieval.py	`24.17% <0.00%> (-75.83%)`	⬇️
...ests/integration/e2e/test_python_feature_server.py	`28.35% <0.00%> (-71.65%)`	⬇️
sdk/python/feast/wait.py	`23.52% <0.00%> (-70.59%)`	⬇️
...gration/registration/test_feature_service_apply.py	`31.25% <0.00%> (-68.75%)`	⬇️
...dk/python/tests/integration/e2e/test_validation.py	`32.72% <0.00%> (-67.28%)`	⬇️
... and 72 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d6fa94...5f6e645. Read the comment docs.

achals · 2022-05-16T20:47:28Z

Hey @tpvasconcelos this looks great! One nit is that this should probably be a fix or a chore instead of a feat (the tooling we're using doesn't allow us to cherry pick feats onto patch releases)

tpvasconcelos · 2022-05-16T20:51:11Z

@achals fixed! Let's keep #2696 open for now? I'll try finding some time to take a look at other cases

achals · 2022-05-16T20:52:55Z

Works for me - I'll move it to a draft state so that it doesn't show up on others ppls review dashboard.

tpvasconcelos · 2022-05-16T20:55:07Z

Ah right you want to include everything in a single PR. I'll update the title and description then 👍

achals · 2022-05-16T21:06:57Z

Ah right you want to include everything in a single PR. I'll update the title and description then 👍

Oh oops! I misunderstood. I am okay merging this in as-is, and keeping the associated ticket open until you feel we have squashed enough of these errors. Please lemme know what you'd like to do!

…py (for better pydantic error messages) Signed-off-by: Tomas Pereira de Vasconcelos <tomasvasconcelos1@gmail.com>

adchia

/lgtm

feast-ci-bot · 2022-08-03T13:55:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adchia, tpvasconcelos

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adchia]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

# [0.24.0](v0.23.0...v0.24.0) (2022-08-05) ### Bug Fixes * More explicit error messages ([feast-dev#2708](https://github.com/felixwang9817/feast/issues/2708)) ([e4d7afd](e4d7afd)) ### Features * Empty file ([e12b33c](e12b33c))

Add more explicit error messages to validation checks in repo_config.py (for better pydantic error messages) Signed-off-by: Tomas Pereira de Vasconcelos <tomasvasconcelos1@gmail.com>

## [0.23.1](v0.23.0...v0.23.1) (2022-08-10) ### Bug Fixes * Check if on_demand_feature_views is an empty list rather than None for snowflake provider ([#3046](#3046)) ([6af401f](6af401f)) * Fix Feast Java inconsistency with int64 serialization vs python ([#3031](#3031)) ([f340aeb](f340aeb)) * Fixing Web UI, which fails for the SQL registry ([#3028](#3028)) ([d584ecd](d584ecd)) * More explicit error messages ([#2708](#2708)) ([ca48963](ca48963)) * Return an empty infra object from sql registry when it doesn't exist ([#3022](#3022)) ([9a64e77](9a64e77))

## [0.22.3](v0.22.2...v0.22.3) (2022-08-10) ### Bug Fixes * Check if on_demand_feature_views is an empty list rather than None for snowflake provider ([#3046](#3046)) ([67af727](67af727)) * Fixing Web UI, which fails for the SQL registry ([#3028](#3028)) ([56d645c](56d645c)) * More explicit error messages ([#2708](#2708)) ([93c1c15](93c1c15)) * Move gcp back to 1.47.0 since grpcio-tools 1.48.0 got yanked from pypi ([#2990](#2990)) ([f7e44da](f7e44da)) * Return an empty infra object from sql registry when it doesn't exist ([#3022](#3022)) ([329bc47](329bc47))

# [0.24.0](v0.23.0...v0.24.0) (2022-08-25) ### Bug Fixes * Check if on_demand_feature_views is an empty list rather than None for snowflake provider ([#3046](#3046)) ([9b05e65](9b05e65)) * FeatureStore.apply applies BatchFeatureView correctly ([#3098](#3098)) ([41be511](41be511)) * Fix Feast Java inconsistency with int64 serialization vs python ([#3031](#3031)) ([4bba787](4bba787)) * Fix feature service inference logic ([#3089](#3089)) ([4310ed7](4310ed7)) * Fix field mapping logic during feature inference ([#3067](#3067)) ([cdfa761](cdfa761)) * Fix incorrect on demand feature view diffing and improve Java tests ([#3074](#3074)) ([0702310](0702310)) * Fix Java helm charts to work with refactored logic. Fix FTS image ([#3105](#3105)) ([2b493e0](2b493e0)) * Fix on demand feature view output in feast plan + Web UI crash ([#3057](#3057)) ([bfae6ac](bfae6ac)) * Fix release workflow to release 0.24.0 ([#3138](#3138)) ([a69aaae](a69aaae)) * Fix Spark offline store type conversion to arrow ([#3071](#3071)) ([b26566d](b26566d)) * Fixing Web UI, which fails for the SQL registry ([#3028](#3028)) ([64603b6](64603b6)) * Force Snowflake Session to Timezone UTC ([#3083](#3083)) ([9f221e6](9f221e6)) * Make infer dummy entity join key idempotent ([#3115](#3115)) ([1f5b1e0](1f5b1e0)) * More explicit error messages ([#2708](#2708)) ([e4d7afd](e4d7afd)) * Parse inline data sources ([#3036](#3036)) ([c7ba370](c7ba370)) * Prevent overwriting existing file during `persist` ([#3088](#3088)) ([69af21f](69af21f)) * Register BatchFeatureView in feature repos correctly ([#3092](#3092)) ([b8e39ea](b8e39ea)) * Return an empty infra object from sql registry when it doesn't exist ([#3022](#3022)) ([8ba87d1](8ba87d1)) * Teardown tables for Snowflake Materialization testing ([#3106](#3106)) ([0a0c974](0a0c974)) * UI error when saved dataset is present in registry. ([#3124](#3124)) ([83cf753](83cf753)) * Update sql.py ([#3096](#3096)) ([2646a86](2646a86)) * Updated snowflake template ([#3130](#3130)) ([f0594e1](f0594e1)) ### Features * Add authentication option for snowflake connector ([#3039](#3039)) ([74c75f1](74c75f1)) * Add Cassandra/AstraDB online store contribution ([#2873](#2873)) ([feb6cb8](feb6cb8)) * Add Snowflake materialization engine ([#2948](#2948)) ([f3b522b](f3b522b)) * Adding saved dataset capabilities for Postgres ([#3070](#3070)) ([d3253c3](d3253c3)) * Allow passing repo config path via flag ([#3077](#3077)) ([0d2d951](0d2d951)) * Contrib azure provider with synapse/mssql offline store and Azure registry store ([#3072](#3072)) ([9f7e557](9f7e557)) * Custom Docker image for Bytewax batch materialization ([#3099](#3099)) ([cdd1b07](cdd1b07)) * Feast AWS Athena offline store (again) ([#3044](#3044)) ([989ce08](989ce08)) * Implement spark offline store `offline_write_batch` method ([#3076](#3076)) ([5b0cc87](5b0cc87)) * Initial Bytewax materialization engine ([#2974](#2974)) ([55c61f9](55c61f9)) * Refactor feature server helm charts to allow passing feature_store.yaml in environment variables ([#3113](#3113)) ([85ee789](85ee789))

feast-ci-bot added the size/S label May 16, 2022

tpvasconcelos changed the title ~~More explicit error messages (repo_config.py)~~ feat: More explicit error messages (repo_config.py) May 16, 2022

tpvasconcelos changed the title ~~feat: More explicit error messages (repo_config.py)~~ fix: More explicit error messages (repo_config.py) May 16, 2022

achals marked this pull request as draft May 16, 2022 20:53

feast-ci-bot added the do-not-merge/work-in-progress label May 16, 2022

tpvasconcelos changed the title ~~fix: More explicit error messages (repo_config.py)~~ fix: More explicit error messages May 16, 2022

adchia marked this pull request as ready for review August 3, 2022 13:54

feast-ci-bot removed the do-not-merge/work-in-progress label Aug 3, 2022

Add more explicit error messages to validation checks in repo_config.…

7b8f8e3

…py (for better pydantic error messages) Signed-off-by: Tomas Pereira de Vasconcelos <tomasvasconcelos1@gmail.com>

adchia force-pushed the explicit-pydantic-errors branch from 5f6e645 to 7b8f8e3 Compare August 3, 2022 13:55

adchia approved these changes Aug 3, 2022

View reviewed changes

feast-ci-bot assigned adchia Aug 3, 2022

feast-ci-bot added lgtm approved labels Aug 3, 2022

feast-ci-bot merged commit e4d7afd into feast-dev:master Aug 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: More explicit error messages #2708

fix: More explicit error messages #2708

tpvasconcelos commented May 16, 2022 •

edited

Loading

tpvasconcelos commented May 16, 2022

codecov-commenter commented May 16, 2022 •

edited

Loading

achals commented May 16, 2022

tpvasconcelos commented May 16, 2022

achals commented May 16, 2022

tpvasconcelos commented May 16, 2022

achals commented May 16, 2022

adchia left a comment

feast-ci-bot commented Aug 3, 2022

fix: More explicit error messages #2708

fix: More explicit error messages #2708

Conversation

tpvasconcelos commented May 16, 2022 • edited Loading

tpvasconcelos commented May 16, 2022

codecov-commenter commented May 16, 2022 • edited Loading

Codecov Report

achals commented May 16, 2022

tpvasconcelos commented May 16, 2022

achals commented May 16, 2022

tpvasconcelos commented May 16, 2022

achals commented May 16, 2022

adchia left a comment

Choose a reason for hiding this comment

feast-ci-bot commented Aug 3, 2022

tpvasconcelos commented May 16, 2022 •

edited

Loading

codecov-commenter commented May 16, 2022 •

edited

Loading