Visitar URL original
feat: Add skip_feature_view_validation parameter to push() for ODFVs with missing UDF dependencies by Copilot · Pull Request #5894 · feast-dev/feast · GitHub
Skip to content

feat: Add skip_feature_view_validation parameter to push() for ODFVs with missing UDF dependencies #5894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

feat: Add skip_feature_view_validation parameter to push() for ODFVs with missing UDF dependencies #5894

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ee8a7d0
Initial plan
Copilot Jan 23, 2026
6b2bf7f
Add skip_validation parameter to push method and related methods
Copilot Jan 23, 2026
f9542e5
Fix caching decorator compatibility with skip_udf parameter
Copilot Jan 23, 2026
79d6744
Apply code formatting with ruff
Copilot Jan 23, 2026
a735bfb
Use explicit identity functions instead of lambda for better type safety
Copilot Jan 23, 2026
2433df6
Only skip UDF validation for ODFVs without write_to_online_store
Copilot Jan 23, 2026
72c471e
Minor wording improvement in test documentation
Copilot Jan 23, 2026
885536e
Rename skip_validation to skip_feature_view_validation for API consis…
Copilot Jan 24, 2026
d4dd5f5
Merge branch 'master' into copilot/fix-module-not-found-error
franciscojavierarceo Jan 25, 2026
98f92a2
Merge branch 'master' into copilot/fix-module-not-found-error
franciscojavierarceo Jan 26, 2026
c111fba
Merge branch 'master' into copilot/fix-module-not-found-error
franciscojavierarceo Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Apply code formatting with ruff 
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
  • Loading branch information
@franciscojavierarceo
Copilot and franciscojavierarceo committed Jan 23, 2026
commit 79d67447277f25e926626ecc5f90a560edb15e52
8 changes: 6 additions & 2 deletions sdk/python/feast/feature_store.py
View file
Open in desktop

@devin-ai-integration devin-ai-integration Bot Feb 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Wrong exception type caught - KeyError vs FeatureViewNotFoundException

The code catches FeatureViewNotFoundException but dictionary key access raises KeyError when the key is missing.

Click to expand

Explanation

At sdk/python/feast/feature_store.py:1993-1996, the code builds a dictionary from feature views and then tries to access it by name:

feature_view_dict = {
    fv_proto.name: fv_proto
    for fv_proto in self.list_all_feature_views(...)
}
try:
    feature_view = feature_view_dict[feature_view_name]
except FeatureViewNotFoundException:
    raise FeatureViewNotFoundException(feature_view_name, self.project)

Actual vs Expected Behavior

  • Actual: When feature_view_name is not in the dictionary, Python raises KeyError, which is NOT caught by the except FeatureViewNotFoundException block. The KeyError propagates up with an unhelpful error message.
  • Expected: The code should catch KeyError and re-raise it as FeatureViewNotFoundException with proper project context.

Impact

When a user calls write_to_online_store(), write_to_online_store_async(), or push() with skip_feature_view_validation=True and an invalid feature view name, they get a confusing KeyError instead of the intended FeatureViewNotFoundException with helpful context about the project.

(Refers to lines 1995-1996)

Recommendation: Change except FeatureViewNotFoundException: to except KeyError: to properly catch the dictionary key lookup failure.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,9 @@ def list_all_feature_views(
Returns:
A list of feature views.
"""
return self._list_all_feature_views(allow_cache, tags=tags, skip_validation=skip_validation)
return self._list_all_feature_views(
allow_cache, tags=tags, skip_validation=skip_validation
)

def list_feature_views(
self, allow_cache: bool = False, tags: Optional[dict[str, str]] = None
Expand Down Expand Up @@ -1966,7 +1968,9 @@ def _get_feature_view_and_df_for_online_write(
):
feature_view_dict = {
fv_proto.name: fv_proto
for fv_proto in self.list_all_feature_views(allow_registry_cache, skip_validation=skip_validation)
for fv_proto in self.list_all_feature_views(
allow_registry_cache, skip_validation=skip_validation
)
}
try:
feature_view = feature_view_dict[feature_view_name]
Expand Down
8 changes: 6 additions & 2 deletions sdk/python/feast/infra/registry/proto_registry_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,9 @@ def list_all_feature_views(
return (
list_feature_views(registry_proto, project, tags)
+ list_stream_feature_views(registry_proto, project, tags)
+ list_on_demand_feature_views(registry_proto, project, tags, skip_udf=skip_udf)
+ list_on_demand_feature_views(
registry_proto, project, tags, skip_udf=skip_udf
)
)
else:
return _list_all_feature_views_cached(registry_proto, project, tags)
Expand Down Expand Up @@ -303,7 +305,9 @@ def list_on_demand_feature_views(
on_demand_feature_view.spec.tags, tags
):
on_demand_feature_views.append(
OnDemandFeatureView.from_proto(on_demand_feature_view, skip_udf=skip_udf)
OnDemandFeatureView.from_proto(
on_demand_feature_view, skip_udf=skip_udf
)
)
return on_demand_feature_views
else:
Expand Down
31 changes: 17 additions & 14 deletions sdk/python/tests/unit/test_skip_validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@

from feast.feature_store import FeatureStore
from feast.on_demand_feature_view import PandasTransformation, PythonTransformation
from feast.protos.feast.core.Transformation_pb2 import UserDefinedFunctionV2 as UserDefinedFunctionProto
from feast.protos.feast.core.Transformation_pb2 import (
UserDefinedFunctionV2 as UserDefinedFunctionProto,
)


def test_apply_has_skip_feature_view_validation_parameter():
Expand Down Expand Up @@ -93,32 +95,33 @@ def test_push_async_has_skip_validation_parameter():

def test_pandas_transformation_from_proto_with_skip_udf():
"""Test that PandasTransformation.from_proto works with skip_udf=True."""

# Create a UDF that would reference a non-existent module
def udf_with_missing_module(df: pd.DataFrame) -> pd.DataFrame:
# This would normally fail if a module is missing during deserialization
import nonexistent_module # noqa: F401

return df

# Serialize the UDF
serialized_udf = dill.dumps(udf_with_missing_module)
udf_string = "import nonexistent_module\ndef udf(df): return df"

# Create proto
udf_proto = UserDefinedFunctionProto(
name="test_udf",
body=serialized_udf,
body_text=udf_string,
)

# Test that skip_udf=True doesn't try to deserialize the UDF
# This would normally fail with ModuleNotFoundError
transformation = PandasTransformation.from_proto(udf_proto, skip_udf=True)

# Should get a dummy transformation with identity function
assert transformation is not None
assert transformation.udf_string == udf_string

# The dummy UDF should be callable and act as identity
test_df = pd.DataFrame({"col1": [1, 2, 3]})
result = transformation.udf(test_df)
Expand All @@ -127,32 +130,33 @@ def udf_with_missing_module(df: pd.DataFrame) -> pd.DataFrame:

def test_python_transformation_from_proto_with_skip_udf():

@franciscojavierarceo franciscojavierarceo Jan 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot we need to validate that the skip_udf is being applied only during push() for ODFVs that do not have write_to_online_store=True because that would hide legitimate breaks.

Copilot AI Jan 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 2433df6. Added logic to only skip UDF deserialization for ODFVs with write_to_online_store=False. ODFVs with write_to_online_store=True always load the actual UDF since it will be executed during push operations. Also added test test_skip_validation_only_applies_to_non_writing_odfvs() to validate this behavior.

"""Test that PythonTransformation.from_proto works with skip_udf=True."""

# Create a UDF that would reference a non-existent module
def udf_with_missing_module(features_dict):
# This would normally fail if a module is missing during deserialization
import nonexistent_module # noqa: F401

return features_dict

# Serialize the UDF
serialized_udf = dill.dumps(udf_with_missing_module)
udf_string = "import nonexistent_module\ndef udf(d): return d"

# Create proto
udf_proto = UserDefinedFunctionProto(
name="test_udf",
body=serialized_udf,
body_text=udf_string,
)

# Test that skip_udf=True doesn't try to deserialize the UDF
# This would normally fail with ModuleNotFoundError
transformation = PythonTransformation.from_proto(udf_proto, skip_udf=True)

# Should get a dummy transformation with identity function
assert transformation is not None
assert transformation.udf_string == udf_string

# The dummy UDF should be callable and act as identity
test_dict = {"col1": 1}
result = transformation.udf(test_dict)
Expand Down Expand Up @@ -212,4 +216,3 @@ def test_skip_validation_use_case_documentation():
- Different teams manage training vs. serving infrastructure
"""
pass # This is a documentation test