| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Thread registry.enable_online_feature_view_versioning through _map_name, which
now delegates to compute_table_id, so that with versioning enabled each feature
view version gets its own Hazelcast map (test_project_driver_stats_v2) instead
of all versions sharing one.
Hazelcast already namespaced as {project}_{table.name}, so this is the same
compute_table_id convention the Milvus and FAISS stores use and the map name is
unchanged when versioning is disabled.
The update() and teardown() SQL statements bind the resolved name to a local
first rather than calling _map_name inside the f-string: line breaks inside an
f-string replacement field are a Python 3.12 feature and this package supports
3.10.
HazelcastOnlineStore is added to the versioned-read allowlist. Its online_read
iterates the requested entity keys and appends (None, None) for a miss, so it
returns one aligned entry per key as the OnlineStore contract requires; a test
pins that property directly.
Also adds Milvus to the VersionedOnlineReadNotSupported message, which has
listed it as unsupported since it was allowlisted.
Part of feast-dev#2728. Closes feast-dev#6174
Signed-off-by: arose26 <145766958+arose26@users.noreply.github.com>
There was a problem hiding this comment.
LGTM. thanks for the contribution
Sorry, something went wrong.
|
⚠️ Please install the Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #6757 +/- ##
==========================================
+ Coverage 47.09% 47.28% +0.19%
==========================================
Files 419 419
Lines 51878 51882 +4
Branches 7525 7525
==========================================
+ Hits 24430 24531 +101
+ Misses 25700 25595 -105
- Partials 1748 1756 +8
Continue to review full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Closes #6174. Part of #2728.
What this does
Threads registry.enable_online_feature_view_versioning through _map_name, which now
delegates to compute_table_id, so that with versioning enabled each feature view version
gets its own Hazelcast map (test_project_driver_stats_v2) instead of all versions sharing
one. Applied to every call site — online_write_batch, online_read, update (both the
keep and delete loops) and teardown.
Hazelcast already namespaced as {project}_{table.name}, so this is the same
compute_table_id convention the merged Milvus (#6330) and FAISS (#6256) stores use — no
deviation. With versioning disabled the map name is byte-identical to today's, pinned by the
versioning=False cases.
One small refactor, and why it is necessary
update() and teardown() previously called _map_name(...) inline inside f-strings:
With the extra argument that call no longer fits on one line, and line breaks inside an
f-string replacement field are a Python 3.12 feature (PEP 701) while this package declares
python_requires=">=3.10". So the resolved name is bound to a local first and the f-strings
just interpolate it. ruff with the project's py310 target rejects the inline form, which
is how I caught it.
Versioned-read allowlist: added, after checking
HazelcastOnlineStore is added to OnlineStore._is_versioned_read_supported(). I read the
path rather than assuming: online_read builds entity_keys_str from the requested keys in
order, then iterates it appending (event_ts, entry) for a hit and (None, None) for a miss —
so it returns one aligned entry per requested key, which is the contract sqlite.py implements
and the property versioned reads depend on. test_read_returns_one_entry_per_requested_key
pins it directly rather than taking my word for it.
Write and read also both serialize entity keys at version 3, so they agree on keys and the
change does not desynchronise them.
Drive-by: a stale error message
VersionedOnlineReadNotSupported listed "SQLite, PostgreSQL, MySQL, FAISS, Redis, and
DynamoDB", but _is_versioned_read_supported() has included MilvusOnlineStore since #6330,
which updated online_store.py without touching errors.py. So the message has been telling
Milvus users their store is unsupported while the code supported it. Added Milvus alongside
Hazelcast — happy to split that into its own PR if you'd prefer.
Tests
New sdk/python/tests/unit/infra/online_store/test_hazelcast_versioning.py, 15 tests,
MagicMock-based in the same style as the merged test_milvus_versioning.py, and
importorskip-guarded on hazelcast.
projection.version_tag takes priority over current_version_number; version 0 gets no
suffix; two versions never collide
versioned map — write is parametrized so the unversioned case is asserted as the control
longer raises, and the alignment property the allowlist rests on is asserted
Verified red-before/green-after: with the three source files reverted, seven of these fail,
while the versioning=False control and the read-alignment test still pass — the latter
correctly, since alignment is a pre-existing property this PR relies on rather than introduces.
Regression check across the 55 unit test files touching online stores or versioning: the set of
failing and erroring tests is identical with and without this change (diff of the sorted
FAILED/ERROR lines is empty). Those pre-existing failures are missing optional dependencies
in my environment. ruff check, ruff format and mypy are clean on all four files.
🤖 Written with Claude Code (Claude Opus 5), reviewed by @arose26.