feat: Add feature view versioning support to PostgreSQL and MySQL online stores#6193
Open
YassinNouh21 wants to merge 1 commit intofeast-dev:masterfrom
Open
feat: Add feature view versioning support to PostgreSQL and MySQL online stores#6193YassinNouh21 wants to merge 1 commit intofeast-dev:masterfrom
YassinNouh21 wants to merge 1 commit intofeast-dev:masterfrom
Conversation
744eaa9 to
b35410f
Compare
…ine stores Add versioned read/write support so that version-qualified feature references (e.g., driver_stats@v2:trips_today) resolve to the correct versioned table in both PostgreSQL and MySQL online stores. Changes: - PostgreSQL: Updated _table_id() and all callers to support enable_versioning - MySQL: Updated _table_id(), _execute_batch(), write_to_table(), and _drop_table_and_index() to thread versioning flag through - online_store.py: Registered PostgreSQLOnlineStore and MySQLOnlineStore in _check_versioned_read_support() - errors.py: Updated VersionedOnlineReadNotSupported message - Unit tests split per store in tests/unit/infra/online_store/ - Integration tests in tests/integration/online_store/ (Docker, testcontainers) Closes feast-dev#6168 Closes feast-dev#6169 Part of feast-dev#2728 Signed-off-by: yassinnouh21 <yassinnouh21@gmail.com>
b35410f to
0e56ae2
Compare
Comment on lines
+259
to
+267
| from feast.infra.online_stores.mysql_online_store.mysql import ( | ||
| MySQLOnlineStore, | ||
| ) | ||
| from feast.infra.online_stores.postgres_online_store.postgres import ( | ||
| PostgreSQLOnlineStore, | ||
| ) | ||
| from feast.infra.online_stores.sqlite import SqliteOnlineStore | ||
|
|
||
| if isinstance(self, SqliteOnlineStore): | ||
| if isinstance( |
Contributor
There was a problem hiding this comment.
🔴 Unconditional import of optional dependencies (pymysql/psycopg) breaks all online reads
The _check_versioned_read_support method unconditionally imports MySQLOnlineStore (which triggers import pymysql) and PostgreSQLOnlineStore (which triggers from psycopg import ...) at the top of the method body. Both pymysql and psycopg are optional extras (defined in pyproject.toml under mysql and postgres groups), not core dependencies. Since _check_versioned_read_support is called unconditionally from both get_online_features (online_store.py:191) and get_online_features_async (online_store.py:320), any user using a different online store (e.g., DynamoDB, Redis, Bigtable) who does not have pymysql or psycopg installed will get an ImportError/ModuleNotFoundError on every online feature read — even non-versioned reads. This completely breaks the online serving path for those users.
Suggested change
| from feast.infra.online_stores.mysql_online_store.mysql import ( | |
| MySQLOnlineStore, | |
| ) | |
| from feast.infra.online_stores.postgres_online_store.postgres import ( | |
| PostgreSQLOnlineStore, | |
| ) | |
| from feast.infra.online_stores.sqlite import SqliteOnlineStore | |
| if isinstance(self, SqliteOnlineStore): | |
| if isinstance( | |
| from feast.infra.online_stores.sqlite import SqliteOnlineStore | |
| supported_types = [SqliteOnlineStore] | |
| try: | |
| from feast.infra.online_stores.mysql_online_store.mysql import ( | |
| MySQLOnlineStore, | |
| ) | |
| supported_types.append(MySQLOnlineStore) | |
| except ImportError: | |
| pass | |
| try: | |
| from feast.infra.online_stores.postgres_online_store.postgres import ( | |
| PostgreSQLOnlineStore, | |
| ) | |
| supported_types.append(PostgreSQLOnlineStore) | |
| except ImportError: | |
| pass | |
| if isinstance(self, tuple(supported_types)): |
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.
Summary
enable_online_feature_view_versioningis enabled, data is routed to version-specific tables (e.g.,project_driver_stats_v2)_check_versioned_read_support()so version-qualified feature references (driver_stats@v2:trips_today) work correctlyChanges
PostgreSQL (
postgres.py):_table_id()to acceptenable_versioningparameter with version suffix logiconline_write_batch,_construct_query_and_params,update,teardown,retrieve_online_documents,retrieve_online_documents_v2MySQL (
mysql.py):_table_id()with same versioning patternenable_versioningthrough_execute_batch(),write_to_table(), and_drop_table_and_index()online_read,update, andteardowncallersShared (
online_store.py,errors.py):PostgreSQLOnlineStoreandMySQLOnlineStoreto_check_versioned_read_support()allowlistVersionedOnlineReadNotSupportederror messageTest plan
_table_id(no version, disabled, enabled, v0, version_tag priority)_table_id(same coverage)_check_versioned_read_support(PostgreSQL/MySQL/SQLite allowed, unsupported raises, no tag no error)Closes #6168
Closes #6169
Part of #2728