Skip to content

Conversation

@franciscojavierarceo
Copy link
Member

@franciscojavierarceo franciscojavierarceo commented Feb 7, 2025

What this PR does / why we need it:

This PR addresses the issue of unexpected crashes when using feature_store.get_online_features with incorrect or missing join keys.

Specifically, it introduces validation logic in the _get_unique_entities function to ensure that all expected join keys are present and non-empty before proceeding.

If any required join keys are missing or have empty values, a KeyError is raised with a descriptive error message. Additionally, new unit tests have been added to cover various edge cases, ensuring that the function behaves as expected under different scenarios.

Changes:

  • sdk/python/feast/utils.py:
    • Added validation for missing and empty join keys in _get_unique_entities.
    • Updated the logic to convert column-oriented table_entity_values into row-wise data and handle edge cases.
  • sdk/python/tests/unit/online_store/test_online_retrieval.py:
    • Added a test case to verify that a KeyError is raised when required join keys are missing.
  • sdk/python/tests/unit/test_unit_feature_store.py:
    • Imported pytest for testing.
    • Renamed test_get_unique_entities to test_get_unique_entities_success for clarity.
    • Added test cases for missing join keys and handling errors appropriately.

Which issue(s) this PR fixes:

#3270

Misc

It's worth noting, when one key is present and one is not, the retrieval will still succeed as this is expected behavior.

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Copy link
Collaborator

@HaoXuAI HaoXuAI left a comment

Choose a reason for hiding this comment

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

LGTM!

@franciscojavierarceo franciscojavierarceo merged commit 3bb0dca into master Feb 8, 2025
35 checks passed
franciscojavierarceo pushed a commit that referenced this pull request Feb 17, 2025
# [0.46.0](v0.45.0...v0.46.0) (2025-02-17)

### Bug Fixes

* Add scylladb to online stores list in docs ([#5061](#5061)) ([08183ed](08183ed))
* Changed feast operator to set status of featurestore cr to ready based on deployment.status = available ([#5020](#5020)) ([fce0d35](fce0d35))
* Ensure Postgres queries are committed or autocommit is used ([#5039](#5039)) ([46f8d7a](46f8d7a))
* Fixing the release workflow to refresh the stable branch when the release is not running in the dry run mode. ([#5057](#5057)) ([a13fa9b](a13fa9b))
* Operator - make onlineStore the default service ([#5044](#5044)) ([6c92447](6c92447))
* Operator - resolve infinite reconciler loop in authz controller ([#5056](#5056)) ([11e4548](11e4548))
* Resolve module on windows ([#4827](#4827)) ([efbffa4](efbffa4))
* Setting the github_token explicitly to see if that solves the problem. ([#5012](#5012)) ([3834ffa](3834ffa))
* Validate entities when running get_online_features ([#5031](#5031)) ([3bb0dca](3bb0dca))

### Features

* Add SQLite retrieve_online_documents_v2 ([#5032](#5032)) ([0fffe21](0fffe21))
* Adding Click command to display configuration details ([#5036](#5036)) ([ae68e4d](ae68e4d))
* Adding volumes and volumeMounts support to Feature Store CR. ([#4983](#4983)) ([ec6f1b7](ec6f1b7))
* Moving the job to seperate action so that we can test it easily. ([#5013](#5013)) ([b9325b7](b9325b7))
* Operator - make server container creation explicit in the CR ([#5024](#5024)) ([b16fb40](b16fb40))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants