Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
| fs_refs = frozenset( | ||
| f"{p.name}:{f.name}" | ||
| for p in fs.feature_view_projections | ||
| for f in p.features | ||
| ) |
There was a problem hiding this comment.
🟡 _resolve_feature_service_name uses p.name instead of p.name_to_use(), causing feature service matching failures
The _resolve_feature_service_name method at sdk/python/feast/feature_store.py:273 constructs feature service refs as f"{p.name}:{f.name}", but the _feature_refs it receives from callers (via utils._get_features at sdk/python/feast/utils.py:1164) are built using f"{projection.name_to_use()}:{f.name}". The name_to_use() method (sdk/python/feast/feature_view_projection.py:52-56) returns self.name_alias or self.name and may append @v{version_tag}. When a feature view projection has an alias or a version tag, the frozensets will never match, so the method silently returns None and the feast.feature_service tag is never set in MLflow for those retrievals.
| fs_refs = frozenset( | |
| f"{p.name}:{f.name}" | |
| for p in fs.feature_view_projections | |
| for f in p.features | |
| ) | |
| fs_refs = frozenset( | |
| f"{p.name_to_use()}:{f.name}" | |
| for p in fs.feature_view_projections | |
| for f in p.features | |
| ) |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
✅ Resolved: The current analysis (ANALYSIS_0001) performed a more thorough trace of all calling paths and determined this is NOT a bug: _resolve_feature_service_name is only called when features is a List[str] (not a FeatureService), and in that code path _get_features returns the user-provided strings as-is. User-provided feature refs use the original FV name (matching p.name), so the comparison is consistent. The previous bug was a false positive.
sdk/python/feast/feature_store.py
Outdated
| f.name: f.dtype.to_value_type() for f in requested_feature_view.features | ||
| } | ||
| return OnlineResponse(online_features_response, feature_types=feature_types) | ||
| return OnlineResponse(online_features_response) |
There was a problem hiding this comment.
🔴 Removal of feature_types from OnlineResponse in document retrieval breaks UUID deserialization
The PR removes the feature_types parameter from three OnlineResponse() calls in retrieve_online_documents and retrieve_online_documents_v2. Previously, feature_types was passed as {f.name: f.dtype.to_value_type() for f in table.features}, and OnlineResponse.to_dict() (sdk/python/feast/online_response.py:76-80) uses this to call feast_value_type_to_python_type(v, feature_type). Without the type hint, the backward-compatibility path at sdk/python/feast/type_map.py:177-186 for UUID/TIME_UUID features stored as string_val will no longer convert them to uuid.UUID objects — they'll be returned as raw strings instead. Other call sites (e.g., sdk/python/feast/infra/online_stores/online_store.py:254) still pass feature_types.
Prompt for agents
The PR removed the feature_types parameter from OnlineResponse() in three places in feature_store.py: retrieve_online_documents (line 3006), and retrieve_online_documents_v2 (lines 3296 and 3319). The old code was:
feature_types = {f.name: f.dtype.to_value_type() for f in requested_feature_view.features} # or table.features
return OnlineResponse(online_features_response, feature_types=feature_types)
This feature_types dict is used in OnlineResponse.to_dict() to enable backward-compatible deserialization of UUID types stored as string_val. Restore the feature_types parameter in these three OnlineResponse() calls to match the behavior of other call sites like online_store.py:254 and online_store.py:406.
Was this helpful? React with 👍 or 👎 to provide feedback.
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
What this PR does / why we need it:
Which issue(s) this PR fixes:
Checks
git commit -s)Testing Strategy
Misc