Skip to content

feat: Add version tracking to FeatureView#6101

Open
franciscojavierarceo wants to merge 14 commits intomasterfrom
featureview-versioning
Open

feat: Add version tracking to FeatureView#6101
franciscojavierarceo wants to merge 14 commits intomasterfrom
featureview-versioning

Conversation

@franciscojavierarceo
Copy link
Member

@franciscojavierarceo franciscojavierarceo commented Mar 12, 2026

Summary

  • Adds version tracking to FeatureView, StreamFeatureView, and OnDemandFeatureView
  • Every feast apply creates a version snapshot in a separate history store (new SQL table / proto field)
  • Users can pin a feature view to a specific historical version declaratively: FeatureView(name="driver_stats", version="v2")
  • By default (version="latest" or omitted), the latest version is always served — fully backward compatible

Changes

Proto layer:

  • New FeatureViewVersion.proto with FeatureViewVersionRecord / FeatureViewVersionHistory
  • Added version field to FeatureViewSpec, StreamFeatureViewSpec, OnDemandFeatureViewSpec
  • Added current_version_number / version_id to meta messages
  • Added feature_view_version_history to Registry proto

Python SDK:

  • New version_utils.py — parses "latest", "v2", "version2" (case-insensitive)
  • New FeatureViewVersionNotFound error class
  • version parameter on FeatureView, StreamFeatureView, OnDemandFeatureView
  • Version-aware apply_feature_view in both SQL registry and file/proto registry
  • list_feature_view_versions(name, project) on registries and FeatureStore
  • CLI: feast feature-views versions <name> subcommand
  • Updated 14 templates with explicit version="latest"

Backward compatibility:

  • version is fully optional — omitting it is identical to version="latest"
  • All new proto fields are additive; old protos deserialize correctly
  • SQL: metadata.create_all(checkfirst=True) auto-creates the new history table
  • Existing idempotency behavior preserved

Test plan

  • 28 unit tests: version parsing, normalization, proto roundtrip, equality, copy
  • 7 integration tests: apply/version creation, idempotency, pinning, error cases
  • Pre-commit hooks pass (format, lint, detect-secrets)
  • CI: existing unit tests pass (no regressions)

🤖 Generated with Claude Code

Resolves #2728


Open with Devin

…emandFeatureView

Every `feast apply` now creates a version snapshot. Users can pin a
feature view to a specific historical version declaratively via
`version="v2"`. By default, the latest version is always served.

- New proto: FeatureViewVersion.proto with version record/history
- Added `version` field to FeatureViewSpec, StreamFeatureViewSpec,
  OnDemandFeatureViewSpec and version metadata to their Meta messages
- New version_utils module for parsing/normalizing version strings
- Version-aware apply_feature_view in both SQL and file registries
- New `list_feature_view_versions` API on FeatureStore and registries
- CLI: `feast feature-views versions <name>` subcommand
- Updated all 14 templates with explicit `version="latest"`
- Unit tests (28) and integration tests (7) for versioning

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@franciscojavierarceo franciscojavierarceo requested a review from a team as a code owner March 12, 2026 20:14
devin-ai-integration[bot]

This comment was marked as resolved.

franciscojavierarceo and others added 2 commits March 12, 2026 16:40
- Fix current_version_number=0 being silently dropped during proto
  deserialization in FeatureView, OnDemandFeatureView (proto3 int32
  default 0 is falsy in Python); use spec.version to disambiguate
- Add current_version_number restoration in StreamFeatureView.from_proto
  (was missing entirely)
- Use timezone-aware UTC datetime in SqlRegistry.list_feature_view_versions
  for consistency with the rest of the codebase
- Add test for v0 proto roundtrip

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Add Versioning section to feature-view.md concept page covering
  automatic snapshots, version pinning, version string formats,
  CLI usage, and Python SDK API
- Add `feast feature-views versions` command to CLI reference

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
devin-ai-integration[bot]

This comment was marked as resolved.

- Fix current_version_number roundtrip bug: version="latest" (always
  truthy) caused None to become 0 after proto roundtrip; now check
  that spec.version is not "latest" before treating 0 as intentional
- Use write_engine (not read_engine) for pre/post apply reads in
  SqlRegistry to avoid read replica lag causing missed version snapshots
- Remove redundant version check in StreamFeatureView.__eq__ (parent
  FeatureView.__eq__ already checks it)
- Add else clause to StreamFeatureView.from_proto for consistency
- Add test for latest/None roundtrip preservation

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
devin-ai-integration[bot]

This comment was marked as resolved.

…ntly

- delete_feature_view now also deletes version history records,
  preventing IntegrityError when re-creating a previously deleted FV
- _get_next_version_number uses write_engine instead of read_engine
  to avoid stale version numbers with read replicas

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
devin-ai-integration[bot]

This comment was marked as resolved.

- Add step-by-step walkthrough showing how versions auto-increment
  on changes and skip on identical re-applies
- Add CLI example showing the apply/change/apply cycle
- Clarify that pinning ignores constructor params and uses the snapshot
- Explain how to return to auto-incrementing after a pin/revert

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

🐛 1 issue in files not directly in the diff

🐛 BatchFeatureView missing version parameter — incomplete transformation (sdk/python/feast/batch_feature_view.py:80-102)

FeatureView, StreamFeatureView, and OnDemandFeatureView all accept a version parameter in their constructors, but BatchFeatureView.__init__ (sdk/python/feast/batch_feature_view.py:80-102) does not. Furthermore, BatchFeatureView.__init__ does not pass version to super().__init__() at sdk/python/feast/batch_feature_view.py:144-158. This means users cannot construct a BatchFeatureView with a pinned version (e.g., BatchFeatureView(..., version="v2") raises TypeError), and any BatchFeatureView created directly always defaults to version="latest". The proto round-trip path works because FeatureView.from_proto sets feature_view.version after construction, but the constructor API is inconsistent with the other feature view types.

View 19 additional findings in Devin Review.

Open in Devin Review

Raises FeatureViewPinConflict when a user pins to an older version
while also modifying the feature view definition (schema, source, etc.).
Fixes FeatureView.__copy__() to include description and owner fields,
which was causing false positive conflict detection.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

🐛 1 issue in files not directly in the diff

🐛 BatchFeatureView missing version parameter — incomplete transformation (sdk/python/feast/batch_feature_view.py:80-102)

FeatureView, StreamFeatureView, and OnDemandFeatureView all accept a version parameter in their constructors, but BatchFeatureView.__init__ (sdk/python/feast/batch_feature_view.py:80-102) does not. Furthermore, BatchFeatureView.__init__ does not pass version to super().__init__() at sdk/python/feast/batch_feature_view.py:144-158. This means users cannot construct a BatchFeatureView with a pinned version (e.g., BatchFeatureView(..., version="v2") raises TypeError), and any BatchFeatureView created directly always defaults to version="latest". The proto round-trip path works because FeatureView.from_proto sets feature_view.version after construction, but the constructor API is inconsistent with the other feature view types.

View 23 additional findings in Devin Review.

Open in Devin Review

franciscojavierarceo and others added 3 commits March 13, 2026 15:14
- 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) <[email protected]>
Clarify that versioning provides definition management and rollback,
not concurrent multi-version serving. Document recommended approaches
(separate projects or distinct FV names) for A/B testing scenarios.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Extends feature view versioning with support for reading features from specific
versions at query time using the syntax: "driver_stats@v2:trips_today"

Core changes:
- Add _parse_feature_ref() to parse version-qualified feature references
- Update all feature reference parsing to use _parse_feature_ref()
- Add get_feature_view_by_version() to BaseRegistry and all implementations
- Add FeatureViewProjection.version_tag for multi-version query support
- Add version-aware _table_id() in SQLite online store (v0→unversioned, v1+→_v{N})
- Add VersionedOnlineReadNotSupported error for unsupported stores

Features:
- "driver_stats:trips" = "driver_stats@latest:trips" (backward compatible)
- "driver_stats@v2:trips" reads from v2 snapshot using _v2 table suffix
- Multiple versions in same query: ["driver@v1:trips", "driver@v2:daily"]
- Version parameter added to all decorator functions for consistency

Backward compatibility:
- Unversioned table serves as v0, only v1+ get _v{N} suffix
- All existing queries work unchanged
- SQLite-only for now, other stores raise clear error

Documentation:
- Updated feature-view.md with @Version syntax examples
- Updated feature-retrieval.md reference format
- Added version examples to how-to guides

Tests: 47 unit + 11 integration tests pass, no regressions

Co-Authored-By: Claude Sonnet 4 <[email protected]>
@franciscojavierarceo franciscojavierarceo requested a review from a team as a code owner March 14, 2026 18:11
@franciscojavierarceo franciscojavierarceo requested review from ejscribner, robhowley and tokoko and removed request for a team March 14, 2026 18:11
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 5 new potential issues.

View 23 additional findings in Devin Review.

Open in Devin Review

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 14, 2026

Choose a reason for hiding this comment

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

🔴 Version-qualified features are dropped from response when full_feature_names=True

When using version-qualified feature references like driver_stats@v2:trips_today, the requested_result_row_names set is computed by a simple replace(":", "__") on the raw feature ref, producing "driver_stats@v2__trips_today". However, _populate_response_from_feature_data at sdk/python/feast/utils.py:1114 uses clean_table_name = table.projection.name_alias or table.projection.name (which is "driver_stats", without @v2), producing "driver_stats__trips_today" in the response. When full_feature_names=True, _drop_unneeded_columns at sdk/python/feast/utils.py:1029-1032 compares response names against requested_result_row_names and drops any name NOT in the set — so "driver_stats__trips_today" is not found in {"driver_stats@v2__trips_today"} and the feature is incorrectly dropped. This makes version-qualified reads completely broken with full_feature_names=True.

Trace of the mismatch
  1. _feature_refs = ["driver_stats@v2:trips_today"] (from _get_features)
  2. requested_result_row_names = {"driver_stats@v2__trips_today"} (line 1393-1394)
  3. Response feature name added: "driver_stats__trips_today" (line 1114-1118)
  4. _drop_unneeded_columns checks: "driver_stats__trips_today" not in {"driver_stats@v2__trips_today"} → True → feature dropped
Open in Devin Review

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

Comment on lines +721 to +728
# Save new as next version
self._save_version_snapshot(
feature_view.name,
project,
next_ver,
fv_type_str,
new_proto_bytes,
)
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 14, 2026

Choose a reason for hiding this comment

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

🟡 SqlRegistry apply_feature_view version snapshot stores proto with stale current_version_number

In SqlRegistry.apply_feature_view at lines 721-728, new_proto_bytes is read from the database before current_version_number is updated. The new_proto_bytes was written by _apply_object which serialized the FV without the correct current_version_number (since it hadn't been assigned yet). This stale proto is then saved as the version snapshot via _save_version_snapshot. Later at lines 730-743, current_version_number is set and the active proto is updated, but the snapshot in the version history table still contains the wrong current_version_number. When someone later retrieves this version via get_feature_view_by_version, the snapshot's current_version_number in the proto meta will be incorrect (it will have whatever value was there before the update).

Open in Devin Review

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

Comment on lines +700 to +722
new_proto_bytes = feature_view_proto.SerializeToString()
if old_proto_bytes is not None:
# FV changed: save old as a version if first time, then save new
next_ver = self._next_version_number(feature_view.name, project)
if next_ver == 0:
self._save_version_record(
feature_view.name, project, 0, fv_type_str, old_proto_bytes
)
next_ver = 1
self._save_version_record(
feature_view.name, project, next_ver, fv_type_str, new_proto_bytes
)
feature_view.current_version_number = next_ver
feature_view_proto = feature_view.to_proto()
feature_view_proto.spec.project = project
else:
# New FV: save as v0
self._save_version_record(
feature_view.name, project, 0, fv_type_str, new_proto_bytes
)
feature_view.current_version_number = 0
feature_view_proto = feature_view.to_proto()
feature_view_proto.spec.project = project
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 14, 2026

Choose a reason for hiding this comment

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

🟡 File Registry apply_feature_view saves new version snapshot with stale proto bytes (before current_version_number is set)

In Registry.apply_feature_view at sdk/python/feast/infra/registry/registry.py:700-722, the new version snapshot is saved using new_proto_bytes obtained at line 700 which is serialized before current_version_number is updated at line 712. The snapshot saved at line 709-711 for the new version thus contains a stale current_version_number in its proto metadata. This means that retrieving the version snapshot later via get_feature_view_by_version will return a FV whose proto metadata has the wrong version number. The same issue exists in the 'new FV' branch at lines 715-722 where the snapshot is saved from new_proto_bytes before current_version_number = 0 is assigned at line 720.

Open in Devin Review

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

- Fix type inference issues in get_feature_view_by_version()
- Use distinct variable names for different proto types
- Ensure proper type annotations for BaseFeatureView subclasses
@tokoko
Copy link
Collaborator

tokoko commented Mar 15, 2026

I haven't been able to go through the whole thing yet, but iiuc the expectation is that online stores would be expected to handle this by essentially treating fv1@v1 and fv1@v2 as two different feature views, right? (sqlite example infers table name from the version). Just wondering, maybe that's to high a bat for online store implementations. for example, if you have fv1@v1 and fv1@v2 both containing 100 features that differ by a single feature only, the online store would have to keep redundant copies of the other 99 features or do some complicated logic to diff values and deduplicate.

wdyt about concentrating on introducing versioning on feature level instead? tbh, that makes more intuitive sense to me. there's a feature called income that one found a fault with. we enable them to introduce income@v2 and somehow deprecate income@v1. online store can handle income@v1 and income@v2 as separate features, but all the rest of the features are unaffected.

@franciscojavierarceo
Copy link
Member Author

I haven't been able to go through the whole thing yet, but iiuc the expectation is that online stores would be expected to handle this by essentially treating fv1@v1 and fv1@v2 as two different feature views, right? (sqlite example infers table name from the version). Just wondering, maybe that's to high a bat for online store implementations. for example, if you have fv1@v1 and fv1@v2 both containing 100 features that differ by a single feature only, the online store would have to keep redundant copies of the other 99 features or do some complicated logic to diff values and deduplicate.

wdyt about concentrating on introducing versioning on feature level instead? tbh, that makes more intuitive sense to me. there's a feature called income that one found a fault with. we enable them to introduce income@v2 and somehow deprecate income@v1. online store can handle income@v1 and income@v2 as separate features, but all the rest of the features are unaffected.

I spent some time reasoning about feature-level versioning with Claude. My initial reaction was that it's too large of a change and it only works today in a broken sense.

By "in a broken sense" I mean that today we don't really version feature views or features, if someone changes the feature view or the feature, we just overwrite it and lose history. Moreover, we essentially force the materialization to be out of sync. Today, that just ignores the behavior silently.

I like how we've implemented it here (i.e., creating a new table and storing the history of the metadata but allowing for callers to specify exact versions and defaulting to the existing behavior) because it is far more explicit about the challenges of materialization consistency issues when you change feature versions.

So, I don't recommend we do feature-level versioning as I worry it makes materialization very unreliable. We can, in fact, declare feature view level versioning because transformations are a collection of features mapped one-to-one with a table.

Implement optional feature view version metadata in API responses to address
the issue where internal @v2 version syntax was leaking into client responses.

Co-Authored-By: Claude Sonnet 4 <[email protected]>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 28 additional findings in Devin Review.

Open in Devin Review

Comment on lines +536 to +587
"""Update non-version-significant fields without creating new version."""
from feast.feature_view import FeatureView

# Metadata fields
existing_proto.spec.description = updated_fv.description
existing_proto.spec.tags.clear()
existing_proto.spec.tags.update(updated_fv.tags)
existing_proto.spec.owner = updated_fv.owner

# Configuration fields (FeatureView)
if (
hasattr(existing_proto.spec, "ttl")
and hasattr(updated_fv, "ttl")
and updated_fv.ttl
):
if isinstance(updated_fv, FeatureView):
ttl_duration = updated_fv.get_ttl_duration()
if ttl_duration:
existing_proto.spec.ttl.CopyFrom(ttl_duration)
if hasattr(existing_proto.spec, "online"):
existing_proto.spec.online = updated_fv.online
if hasattr(existing_proto.spec, "offline"):
existing_proto.spec.offline = updated_fv.offline
if hasattr(existing_proto.spec, "enable_validation"):
existing_proto.spec.enable_validation = updated_fv.enable_validation

# OnDemandFeatureView configuration
if hasattr(existing_proto.spec, "write_to_online_store"):
existing_proto.spec.write_to_online_store = updated_fv.write_to_online_store
if hasattr(existing_proto.spec, "singleton"):
existing_proto.spec.singleton = updated_fv.singleton

# Data sources (treat as configuration)
if (
hasattr(existing_proto.spec, "batch_source")
and hasattr(updated_fv, "batch_source")
and updated_fv.batch_source
):
existing_proto.spec.batch_source.CopyFrom(
updated_fv.batch_source.to_proto()
)
if (
hasattr(existing_proto.spec, "stream_source")
and hasattr(updated_fv, "stream_source")
and updated_fv.stream_source
):
existing_proto.spec.stream_source.CopyFrom(
updated_fv.stream_source.to_proto()
)

# Update timestamp
existing_proto.meta.last_updated_timestamp.FromDatetime(datetime.utcnow())
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 _update_metadata_fields does not update spec.version, leaving stale version pin in registry

When _update_metadata_fields is called (for metadata-only changes without schema/UDF modifications), it updates description, tags, owner, and configuration fields but does NOT update spec.version. This means if a user transitions from a pinned version (e.g., version="v0") back to version="latest" without making schema changes, the version field in the registry proto remains "v0". Subsequent reads from the registry will show the feature view as pinned to v0, even though the user intended to go back to auto-incrementing mode. The spec.version field should be updated alongside other metadata fields.

Suggested change
"""Update non-version-significant fields without creating new version."""
from feast.feature_view import FeatureView
# Metadata fields
existing_proto.spec.description = updated_fv.description
existing_proto.spec.tags.clear()
existing_proto.spec.tags.update(updated_fv.tags)
existing_proto.spec.owner = updated_fv.owner
# Configuration fields (FeatureView)
if (
hasattr(existing_proto.spec, "ttl")
and hasattr(updated_fv, "ttl")
and updated_fv.ttl
):
if isinstance(updated_fv, FeatureView):
ttl_duration = updated_fv.get_ttl_duration()
if ttl_duration:
existing_proto.spec.ttl.CopyFrom(ttl_duration)
if hasattr(existing_proto.spec, "online"):
existing_proto.spec.online = updated_fv.online
if hasattr(existing_proto.spec, "offline"):
existing_proto.spec.offline = updated_fv.offline
if hasattr(existing_proto.spec, "enable_validation"):
existing_proto.spec.enable_validation = updated_fv.enable_validation
# OnDemandFeatureView configuration
if hasattr(existing_proto.spec, "write_to_online_store"):
existing_proto.spec.write_to_online_store = updated_fv.write_to_online_store
if hasattr(existing_proto.spec, "singleton"):
existing_proto.spec.singleton = updated_fv.singleton
# Data sources (treat as configuration)
if (
hasattr(existing_proto.spec, "batch_source")
and hasattr(updated_fv, "batch_source")
and updated_fv.batch_source
):
existing_proto.spec.batch_source.CopyFrom(
updated_fv.batch_source.to_proto()
)
if (
hasattr(existing_proto.spec, "stream_source")
and hasattr(updated_fv, "stream_source")
and updated_fv.stream_source
):
existing_proto.spec.stream_source.CopyFrom(
updated_fv.stream_source.to_proto()
)
# Update timestamp
existing_proto.meta.last_updated_timestamp.FromDatetime(datetime.utcnow())
def _update_metadata_fields(
self, existing_proto: Message, updated_fv: BaseFeatureView
) -> None:
"""Update non-version-significant fields without creating new version."""
from feast.feature_view import FeatureView
# Metadata fields
existing_proto.spec.description = updated_fv.description
existing_proto.spec.tags.clear()
existing_proto.spec.tags.update(updated_fv.tags)
existing_proto.spec.owner = updated_fv.owner
# Version field
if hasattr(existing_proto.spec, "version") and hasattr(updated_fv, "version"):
existing_proto.spec.version = updated_fv.version
# Configuration fields (FeatureView)
if (
hasattr(existing_proto.spec, "ttl")
and hasattr(updated_fv, "ttl")
and updated_fv.ttl
):
if isinstance(updated_fv, FeatureView):
ttl_duration = updated_fv.get_ttl_duration()
if ttl_duration:
existing_proto.spec.ttl.CopyFrom(ttl_duration)
if hasattr(existing_proto.spec, "online"):
existing_proto.spec.online = updated_fv.online
if hasattr(existing_proto.spec, "offline"):
existing_proto.spec.offline = updated_fv.offline
if hasattr(existing_proto.spec, "enable_validation"):
existing_proto.spec.enable_validation = updated_fv.enable_validation
# OnDemandFeatureView configuration
if hasattr(existing_proto.spec, "write_to_online_store"):
existing_proto.spec.write_to_online_store = updated_fv.write_to_online_store
if hasattr(existing_proto.spec, "singleton"):
existing_proto.spec.singleton = updated_fv.singleton
# Data sources (treat as configuration)
if (
hasattr(existing_proto.spec, "batch_source")
and hasattr(updated_fv, "batch_source")
and updated_fv.batch_source
):
existing_proto.spec.batch_source.CopyFrom(
updated_fv.batch_source.to_proto()
)
if (
hasattr(existing_proto.spec, "stream_source")
and hasattr(updated_fv, "stream_source")
and updated_fv.stream_source
):
existing_proto.spec.stream_source.CopyFrom(
updated_fv.stream_source.to_proto()
)
# Update timestamp
existing_proto.meta.last_updated_timestamp.FromDatetime(datetime.utcnow())
Open in Devin Review

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

Add missing include_feature_view_version_metadata parameter to:
- EmbeddedGoOnlineFeaturesService.get_online_features()
- FooProvider.get_online_features() and get_online_features_async()
- FooProvider.retrieve_online_documents() and retrieve_online_documents_v2()

This resolves CI failures where provider implementations were not updated
with the new parameter from the abstract Provider interface.

Co-Authored-By: Claude Sonnet 4 <[email protected]>
Add missing include_feature_view_version_metadata parameter to:
- SqliteOnlineStore.retrieve_online_documents/v2
- FaissOnlineStore.retrieve_online_documents
- QdrantOnlineStore.retrieve_online_documents
- MilvusOnlineStore.retrieve_online_documents_v2
- RemoteOnlineStore.retrieve_online_documents/v2
- PostgresOnlineStore.retrieve_online_documents/v2
- ElasticsearchOnlineStore.retrieve_online_documents/v2

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 new potential issues.

View 33 additional findings in Devin Review.

Open in Devin Review

Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 Version-qualified feature refs break retrieve_online_documents with FeatureViewNotFoundException

retrieve_online_documents uses _feature.split(":")[0] to extract the feature view name, which for a version-qualified ref like driver_stats@v2:trips yields driver_stats@v2. This is then passed directly to self.get_feature_view("driver_stats@v2"), which looks up by name in the registry and will raise FeatureViewNotFoundException since FVs are stored under their plain name (driver_stats). The same issue exists at sdk/python/feast/feature_store.py:2820 in retrieve_online_documents_v2. The PR introduced version-qualified refs but did not update these two parsing sites to strip the @vN suffix.

(Refers to line 2617)

Prompt for agents
In sdk/python/feast/feature_store.py, two methods parse feature refs using split(":")[0] which doesn't handle @version syntax:

1. In retrieve_online_documents (around line 2617):
   Change `feature_view_name = _feature.split(":")[0]` to use `utils._parse_feature_ref(_feature)[0]` to get the plain FV name.

2. In retrieve_online_documents_v2 (around line 2820):
   Change `feature_view_name = feature.split(":")[0]` to use `utils._parse_feature_ref(feature)[0]` to get the plain FV name.

Also at line 2623 and 2831, `f.split(":")[1]` should use `utils._parse_feature_ref(f)[2]` to correctly extract the feature name from version-qualified refs.
Open in Devin Review

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

Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 _resolve_feature_counts in feature_server.py uses split(":")[0] which includes @version in FV name

In sdk/python/feast/feature_server.py:141, _resolve_feature_counts uses ref.split(":")[0] to extract feature view names for counting. For version-qualified refs like driver_stats@v2:trips, this produces driver_stats@v2 rather than driver_stats. This means the same feature view queried at two different versions would be counted as two separate feature views, inflating the fv_count metric.

(Refers to line 141)

Open in Devin Review

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

Comment on lines +737 to 740
self._update_metadata_fields(
existing_feature_view_proto, feature_view
)
return
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 Metadata-only updates are not committed to the registry store

When apply_feature_view detects that only metadata changed (not schema/UDF), _update_metadata_fields modifies the in-memory proto at sdk/python/feast/infra/registry/registry.py:737-739, then immediately returns at line 740 without calling self.commit(). This means description, tags, owner, TTL, online/offline flag changes are applied to cached_registry_proto in memory but never persisted to the backing store. The next time the registry is loaded from storage, these changes are lost.

Old vs new behavior

The old code returned early only when the FV was completely identical (== feature_view), so there was nothing to persist. The new code returns early for metadata-only changes after mutating the proto, but skips the if commit: self.commit() block at line 785-786.

Suggested change
self._update_metadata_fields(
existing_feature_view_proto, feature_view
)
return
# Update non-version-significant fields in place
self._update_metadata_fields(
existing_feature_view_proto, feature_view
)
if commit:
self.commit()
return
Open in Devin Review

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improved feature view and model versioning

2 participants