Fix cache overwrite during batch feature view apply#6211
Open
SARAMALI15792 wants to merge 2 commits intofeast-dev:masterfrom
Open
Fix cache overwrite during batch feature view apply#6211SARAMALI15792 wants to merge 2 commits intofeast-dev:masterfrom
SARAMALI15792 wants to merge 2 commits intofeast-dev:masterfrom
Conversation
added 2 commits
April 1, 2026 14:00
The file-based registry was incorrectly checking for feature view name conflicts across all projects in a shared registry, causing false positives when different projects used the same feature view names. This fix replaces the custom conflict checking logic with the base class method _ensure_feature_view_name_is_unique, which properly scopes checks by project. This aligns the file registry behavior with the SQL registry implementation. Changes: - Use _ensure_feature_view_name_is_unique from BaseRegistry (same as SQL registry) - Remove _check_conflicting_feature_view_names and _existing_feature_view_names_to_fvs methods - Add regression test for cross-project feature view names Fixes feast-dev#6209
Pass allow_cache=True to _ensure_feature_view_name_is_unique to prevent cache overwrites when applying multiple feature views with commit=False. The issue occurred because _ensure_feature_view_name_is_unique calls getter methods with allow_cache=False by default, which triggers _get_registry_proto to re-read from the store and overwrite cached_registry_proto, discarding uncommitted in-memory changes. This was triggered in feature_store.py where apply_feature_view is called in a loop with commit=False after other uncommitted apply_project and apply_data_source calls. Each uniqueness check would overwrite the cache, losing prior uncommitted changes. Since _prepare_registry_for_changes already prepares the cache at line 755, using allow_cache=True is safe and preserves uncommitted state during batch operations.
Comment on lines
+2197
to
+2200
| @pytest.mark.parametrize( | ||
| "test_registry", | ||
| all_fixtures, | ||
| ) |
Contributor
There was a problem hiding this comment.
🟡 New test missing @pytest.mark.integration marker, breaking test categorization
The new test_cross_project_feature_view_name_allowed test is missing the @pytest.mark.integration marker that every other parametrized test using all_fixtures in this file has (e.g., test_cross_type_feature_view_name_conflict at line 2081, test_cross_type_feature_view_odfv_conflict at line 2150, and all ~20 other tests). Without this marker, the test won't be selected when running integration tests via make test-python-integration (which filters by pytest.mark.integration), so the regression test for issue #6209 will never actually run in CI.
Suggested change
| @pytest.mark.parametrize( | |
| "test_registry", | |
| all_fixtures, | |
| ) | |
| @pytest.mark.integration | |
| @pytest.mark.parametrize( | |
| "test_registry", | |
| all_fixtures, | |
| ) |
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the cache overwrite issue identified in #6210
Summary
This PR fixes a critical bug where
_ensure_feature_view_name_is_uniquewas causing cache overwrites during batch feature view apply operations, resulting in loss of uncommitted in-memory changes.Problem
The original fix in #6210 introduced a regression where calling
_ensure_feature_view_name_is_uniquewith the defaultallow_cache=Falseparameter caused the getter methods to re-read from the registry store and overwriteself.cached_registry_proto, discarding any uncommitted changes.This was triggered in
feature_store.py:1199-1202whereapply_feature_viewis called in a loop withcommit=Falseafter other uncommittedapply_projectandapply_data_sourcecalls:Each uniqueness check would overwrite the cache, losing:
Solution
Pass
allow_cache=Trueto_ensure_feature_view_name_is_uniqueat line 758. This is safe because:_prepare_registry_for_changesalready prepares the cache at line 755Changes
sdk/python/feast/infra/registry/registry.py:758: Addallow_cache=TrueparameterTest Plan
Addresses the review feedback from #6210 (comment)