Skip to content

fix: use ValueError and fix f-string interpolation in yaml_loader#6263

Closed
korbonits wants to merge 1 commit intofeast-dev:masterfrom
korbonits:fix/yaml-loader-exceptions
Closed

fix: use ValueError and fix f-string interpolation in yaml_loader#6263
korbonits wants to merge 1 commit intofeast-dev:masterfrom
korbonits:fix/yaml-loader-exceptions

Conversation

@korbonits
Copy link
Copy Markdown

@korbonits korbonits commented Apr 11, 2026

What this PR does / why we need it:

Fixes two issues in sdk/python/feast/loaders/yaml.py:

  1. Broken f-string interpolation: ${yaml_strings} in the error message on line 25 does not interpolate in Python — the $ prefix is invalid and the variable value was never included in the error string. Fixed to {yaml_strings}.
  2. Over-broad exception types: Both yaml_loader and _get_yaml_contents raised bare Exception instead of ValueError, making it harder for callers to catch specific errors.

Which issue(s) this PR fixes:

N/A

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

Replace bare Exception with ValueError in yaml_loader and
_get_yaml_contents for more specific error handling. Also fix
a broken f-string interpolation (${yaml_strings} → {yaml_strings})
in the error message.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Alex Korbonits <alex@korbonits.com>
@korbonits korbonits requested a review from a team as a code owner April 11, 2026 17:29
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@korbonits korbonits marked this pull request as draft April 11, 2026 18:10
@korbonits korbonits marked this pull request as ready for review April 11, 2026 18:40
@korbonits
Copy link
Copy Markdown
Author

Sorry this is dead code, going to close this PR. Maybe I will raise another to delete it.

@korbonits korbonits closed this Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant