Skip to content

fix: Check duplicate names for feature view across types #5999

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

Merged
ntkathole merged 5 commits into from
Mar 6, 2026
Merged

fix: Check duplicate names for feature view across types #5999

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6e256b6
check duplicate names for feature view across types
Prathap-P Feb 21, 2026
e319a6f
fix: check uniqueness before applying feature view
Prathap-P Feb 21, 2026
be0e8a3
use FeatureViewNotFoundException for all types as per Devin's suggestion
Prathap-P Mar 4, 2026
8363cd4
fix tests
Prathap-P Mar 6, 2026
d7bac33
fix linting issues
Prathap-P Mar 6, 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
use FeatureViewNotFoundException for all types as per Devin's suggestion 
Signed-off-by: Prathap P <436prathap@gmail.com>
  • Loading branch information
@Prathap-P @ntkathole
Prathap-P authored and ntkathole committed Mar 6, 2026
commit be0e8a3662f8f7fb565ce4c91b250e76d6e9efc1
10 changes: 0 additions & 10 deletions sdk/python/feast/errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,6 @@ def __init__(self, name, project=None):
super().__init__(f"On demand feature view {name} does not exist")


class StreamFeatureViewNotFoundException(FeastObjectNotFoundException):
def __init__(self, name, project=None):
if project:
super().__init__(
f"Stream feature view {name} does not exist in project {project}"
)
else:
super().__init__(f"Stream feature view {name} does not exist")


class RequestDataNotFoundInEntityDfException(FeastObjectNotFoundException):
def __init__(self, feature_name, feature_view_name):
super().__init__(
Expand Down
11 changes: 5 additions & 6 deletions sdk/python/feast/infra/registry/base_registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
from feast.errors import (
ConflictingFeatureViewNames,
FeatureViewNotFoundException,
OnDemandFeatureViewNotFoundException,
StreamFeatureViewNotFoundException,
)
from feast.feature_service import FeatureService
from feast.feature_view import FeatureView
Expand Down Expand Up @@ -293,24 +291,25 @@ def _check_conflict(getter, not_found_exc, existing_type: str):
pass
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.

# Check StreamFeatureView before FeatureView since StreamFeatureView is a subclass
# Note: All getters raise FeatureViewNotFoundException (not type-specific exceptions)
if isinstance(feature_view, StreamFeatureView):
Copy link
Copy Markdown
Collaborator

@HaoXuAI HaoXuAI Feb 22, 2026

Choose a reason for hiding this comment

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

Does this mean it will call to retrieve feature view many times? Can we optimize it?

Copy link
Copy Markdown
Contributor Author

@Prathap-P Prathap-P Feb 24, 2026

Choose a reason for hiding this comment

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

If I understand correctly, you're concerned that when creating 100 feature views, the apply_feature_view function calls the getters for the other two types each time. That results in 2 database calls per feature view — 200 total — and you’re wondering if we should optimize this part.

Copy link
Copy Markdown
Contributor Author

@Prathap-P Prathap-P Feb 24, 2026

Choose a reason for hiding this comment

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

@HaoXuAI
I have a plan. We need to validate in two places:

  1. In the incoming request — ensure the new feature view name doesn’t exist in the other two types within the request (and vice versa).
  2. In the database — check against existing records (for the SQL registry case).

Currently, the cache util doesn’t have an exists function to check if a name is already in the registry. Even if it did, calling it 100 times wouldn’t be efficient.

So I’m planning to create a util function per feature view type that takes a list of names and a project, and checks that none of the names exist in the DB.

For example, if 50 feature views come in:

  • I’ll call the new util once with those 50 names against the stream and on-demand tables.
  • Do the same vice versa for the other types.

This way, instead of calling per feature view, we make only two DB calls per type.

That’s my current idea. Do you see any better optimization?

Copy link
Copy Markdown
Contributor Author

@Prathap-P Prathap-P Feb 25, 2026

Choose a reason for hiding this comment

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

@HaoXuAI I can go ahead to implement, if you find we are good with above logic 😄

Copy link
Copy Markdown
Collaborator

@HaoXuAI HaoXuAI Feb 28, 2026

Choose a reason for hiding this comment

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

i think that makes sense. Feel free to create a new issue for this. We can go with the current solution for now

_check_conflict(
self.get_feature_view, FeatureViewNotFoundException, "FeatureView"
)
_check_conflict(
self.get_on_demand_feature_view,
OnDemandFeatureViewNotFoundException,
FeatureViewNotFoundException,
"OnDemandFeatureView",
)
elif isinstance(feature_view, FeatureView):
_check_conflict(
self.get_stream_feature_view,
StreamFeatureViewNotFoundException,
FeatureViewNotFoundException,
"StreamFeatureView",
)
_check_conflict(
self.get_on_demand_feature_view,
OnDemandFeatureViewNotFoundException,
FeatureViewNotFoundException,
"OnDemandFeatureView",
)
elif isinstance(feature_view, OnDemandFeatureView):
Expand All @@ -319,7 +318,7 @@ def _check_conflict(getter, not_found_exc, existing_type: str):
)
_check_conflict(
self.get_stream_feature_view,
StreamFeatureViewNotFoundException,
FeatureViewNotFoundException,
"StreamFeatureView",
)
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.

Expand Down