| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
⚠️ Please install the Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #6820 +/- ##
==========================================
- Coverage 47.08% 47.08% -0.01%
==========================================
Files 419 419
Lines 51877 51872 -5
Branches 7525 7524 -1
==========================================
- Hits 24428 24424 -4
+ Misses 25700 25699 -1
Partials 1749 1749
*This pull request uses carry forward flags. Click here to find out more.
Continue to review full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
Three empty-input shapes reached internal exceptions, and the feature server
turned each into an HTTP 500:
entity_rows=[] -> IndexError: list index out of range
entities={"driver_id": []} -> KeyError: Missing join key values for keys: []
entities={} -> KeyError: 'pop from an empty set'
The middle case is a well-formed request. The join key was supplied, it simply
has no values, which is what a caller sends when the upstream query matched
nothing that run.
get_online_features built its columnar dict from entity_rows[0] on both the
sync and async paths, so an empty list raised before anything else ran.
_validate_entity_values ended with set_of_row_lengths.pop(); for a mapping with
no columns the set is empty and pop() raised. Report zero rows instead.
_get_unique_entities treated a join key present with zero values the same as a
join key never supplied, even though the row-wise conversion just below it
already returns empty results for that case. Raise only when nothing at all was
supplied for the view, which leaves the existing behaviour intact: a partially
supplied key set still proceeds, and a caller that supplied nothing relevant
still errors with the same message listing the expected keys.
That error is now MissingJoinKeyValuesException, carrying HTTP 400 so the server
reports a client error rather than a 500. It subclasses KeyError as well, since
that is what this condition raised before and callers catch it.
Signed-off-by: Daksha1611 <mehtadaksha1611@gmail.com>
The guard was added to both get_online_features and its async twin, but only the sync one had a test. Without the fix the async case raises IndexError: list index out of range. Signed-off-by: Daksha1611 <mehtadaksha1611@gmail.com>
| Back | FazBrowse Home | New Git URL |
What this PR does / why we need it:
Three empty-input shapes reached internal exceptions, and the feature server turned each
into an HTTP 500:
entity_rows=[] -> IndexError: list index out of range entities={"driver_id": []} -> KeyError: Missing join key values for keys: [] entities={} -> KeyError: 'pop from an empty set'The middle case is a well-formed request: the join key was supplied, it simply holds
no values, which is what a caller sends when the upstream query matched nothing that run.
Three causes, fixed here:
and async paths, so an empty list raised before anything else ran.
columns the set is empty and pop() raised. It now reports zero rows.
never supplied — even though the row-wise conversion immediately below it already
returns empty results for that case (if not rowise: return (), (), 0). It now raises
only when nothing at all was supplied for the view.
That last change is deliberately narrow, so existing behaviour is preserved: a partially
supplied key set still proceeds (covered by test_get_unique_entities_missing_join_key_success),
and a caller supplying nothing relevant still errors with the same message listing the
expected keys (covered by test_get_unique_entities_missing_all_join_keys_error).
The error is now MissingJoinKeyValuesException, carrying HTTP 400 so the server reports
a client error rather than a 500. It subclasses KeyError as well, since that is what
this condition raised before and callers catch it — so this is not a breaking change.
Resulting behaviour: join keys supplied but empty → 200 with an empty result and correct
feature-name metadata; nothing supplied → 400 naming the missing key.
Which issue(s) this PR fixes:
Fixes #6819
Checks
Testing Strategy
New sdk/python/tests/unit/online_store/test_empty_entity_rows.py. Four of its five
tests fail on master, reproducing all three symptoms above; the fifth asserts the new
exception is still a KeyError and carries 400.
Full sdk/python/tests/unit: 2612 passed, 24 skipped. The 6 failures and 28 errors
in that run are environmental and present on master too — PySpark worker/driver Python
version mismatch, a torch import, MongoDB testcontainers, and a missing CUDA library.
ruff check, ruff format --check and mypy are clean on all changed files.
Misc
An empty response was chosen over a 400 for the empty-but-present case, so batch-scoring
callers don't have to special-case "my filter matched nothing". It also lines up with
get_historical_features returning an empty frame for a zero-row entity_df. Happy to
switch to a 400 if maintainers prefer the stricter reading.