Skip to content

Commit 2069b22

Browse files
franciscojavierarceoclaude
franciscojavierarceo
and
claude
committed
fix: Address Devin review feedback on versioning
- Add version parameter to BatchFeatureView constructor for consistency with FeatureView, StreamFeatureView, and OnDemandFeatureView - Clean up version history records in file registry delete_feature_view to prevent orphaned records on re-creation - Fix current_version_number proto roundtrip: preserve 0 when version="latest" (after first apply) instead of incorrectly returning None Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f9e896f commit 2069b22

File tree

6 files changed

+50
-36
lines changed

6 files changed

+50
-36
lines changed

‎sdk/python/feast/batch_feature_view.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ def __init__(
100100
batch_engine: Optional[Dict[str, Any]] = None,
101101
aggregations: Optional[List[Aggregation]] = None,
102102
enable_validation: bool = False,
103+
version: str = "latest",
103104
):
104105
if not flags_helper.is_test():
105106
warnings.warn(
@@ -155,6 +156,7 @@ def __init__(
155156
sink_source=sink_source,
156157
mode=mode,
157158
enable_validation=enable_validation,
159+
version=version,
158160
)
159161

160162
def get_feature_transformation(self) -> Optional[Transformation]:

‎sdk/python/feast/feature_view.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -646,16 +646,16 @@ def _from_proto_internal(
646646
# Restore version fields.
647647
feature_view.version = feature_view_proto.spec.version or "latest"
648648
# proto3 int32 defaults to 0, so use spec.version to distinguish
649-
# "actually version 0" from "no version set". A version of "latest"
650-
# (or empty) with current_version_number==0 means "not versioned yet".
649+
# "actually version 0" from "no version set". An empty spec.version
650+
# means the proto predates versioning, so current_version_number
651+
# should be None.
651652
if feature_view_proto.meta.current_version_number:
652653
feature_view.current_version_number = (
653654
feature_view_proto.meta.current_version_number
654655
)
655656
elif (
656657
feature_view_proto.meta.current_version_number == 0
657658
and feature_view_proto.spec.version
658-
and feature_view_proto.spec.version.lower() != "latest"
659659
):
660660
feature_view.current_version_number = 0
661661
else:

‎sdk/python/feast/infra/registry/registry.py‎

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,7 @@ def delete_feature_view(self, name: str, project: str, commit: bool = True):
885885
self._prepare_registry_for_changes(project)
886886
assert self.cached_registry_proto
887887

888+
found = False
888889
for idx, existing_feature_view_proto in enumerate(
889890
self.cached_registry_proto.feature_views
890891
):
@@ -893,35 +894,48 @@ def delete_feature_view(self, name: str, project: str, commit: bool = True):
893894
and existing_feature_view_proto.spec.project == project
894895
):
895896
del self.cached_registry_proto.feature_views[idx]
896-
if commit:
897-
self.commit()
898-
return
897+
found = True
898+
break
899899

900-
for idx, existing_on_demand_feature_view_proto in enumerate(
901-
self.cached_registry_proto.on_demand_feature_views
902-
):
903-
if (
904-
existing_on_demand_feature_view_proto.spec.name == name
905-
and existing_on_demand_feature_view_proto.spec.project == project
900+
if not found:
901+
for idx, existing_on_demand_feature_view_proto in enumerate(
902+
self.cached_registry_proto.on_demand_feature_views
906903
):
907-
del self.cached_registry_proto.on_demand_feature_views[idx]
908-
if commit:
909-
self.commit()
910-
return
904+
if (
905+
existing_on_demand_feature_view_proto.spec.name == name
906+
and existing_on_demand_feature_view_proto.spec.project == project
907+
):
908+
del self.cached_registry_proto.on_demand_feature_views[idx]
909+
found = True
910+
break
911911

912-
for idx, existing_stream_feature_view_proto in enumerate(
913-
self.cached_registry_proto.stream_feature_views
914-
):
915-
if (
916-
existing_stream_feature_view_proto.spec.name == name
917-
and existing_stream_feature_view_proto.spec.project == project
912+
if not found:
913+
for idx, existing_stream_feature_view_proto in enumerate(
914+
self.cached_registry_proto.stream_feature_views
918915
):
919-
del self.cached_registry_proto.stream_feature_views[idx]
920-
if commit:
921-
self.commit()
922-
return
916+
if (
917+
existing_stream_feature_view_proto.spec.name == name
918+
and existing_stream_feature_view_proto.spec.project == project
919+
):
920+
del self.cached_registry_proto.stream_feature_views[idx]
921+
found = True
922+
break
923923

924-
raise FeatureViewNotFoundException(name, project)
924+
if not found:
925+
raise FeatureViewNotFoundException(name, project)
926+
927+
# Clean up version history for the deleted feature view
928+
history = self.cached_registry_proto.feature_view_version_history
929+
indices_to_remove = [
930+
i
931+
for i, record in enumerate(history.records)
932+
if record.feature_view_name == name and record.project_id == project
933+
]
934+
for i in reversed(indices_to_remove):
935+
del history.records[i]
936+
937+
if commit:
938+
self.commit()
925939

926940
def delete_entity(self, name: str, project: str, commit: bool = True):
927941
self._prepare_registry_for_changes(project)

‎sdk/python/feast/on_demand_feature_view.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,6 @@ def from_proto(
564564
elif (
565565
on_demand_feature_view_proto.meta.current_version_number == 0
566566
and on_demand_feature_view_proto.spec.version
567-
and on_demand_feature_view_proto.spec.version.lower() != "latest"
568567
):
569568
on_demand_feature_view_obj.current_version_number = 0
570569
else:

‎sdk/python/feast/stream_feature_view.py‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,11 +361,7 @@ def from_proto(cls, sfv_proto):
361361
stream_feature_view.current_version_number = (
362362
sfv_proto.meta.current_version_number
363363
)
364-
elif (
365-
sfv_proto.meta.current_version_number == 0
366-
and sfv_proto.spec.version
367-
and sfv_proto.spec.version.lower() != "latest"
368-
):
364+
elif sfv_proto.meta.current_version_number == 0 and sfv_proto.spec.version:
369365
stream_feature_view.current_version_number = 0
370366
else:
371367
stream_feature_view.current_version_number = None

‎sdk/python/tests/unit/test_feature_view_versioning.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,10 @@ def test_feature_view_proto_roundtrip_v0(self):
174174
assert fv2.version == "v0"
175175
assert fv2.current_version_number == 0
176176

177-
def test_feature_view_proto_roundtrip_latest_none(self):
178-
"""version='latest' with current_version_number=None must not become 0."""
177+
def test_feature_view_proto_roundtrip_latest_zero(self):
178+
"""version='latest' with current_version_number=None becomes 0 after
179+
proto roundtrip because proto3 cannot distinguish unset int32 from 0.
180+
This is acceptable — 0 is the correct initial version number."""
179181
from datetime import timedelta
180182

181183
from feast.entity import Entity
@@ -193,7 +195,8 @@ def test_feature_view_proto_roundtrip_latest_none(self):
193195
proto = fv.to_proto()
194196
fv2 = FeatureView.from_proto(proto)
195197
assert fv2.version == "latest"
196-
assert fv2.current_version_number is None
198+
# proto3 int32 default is 0; with version="latest" set, we preserve 0
199+
assert fv2.current_version_number == 0
197200

198201
def test_feature_view_equality_with_version(self):
199202
from datetime import timedelta

0 commit comments

Comments
 (0)