fix: Repair MySQL online store batch_write path - #6802
Open
karthikchundi-commits wants to merge 1 commit into
Open
fix: Repair MySQL online store batch_write path#6802karthikchundi-commits wants to merge 1 commit into
karthikchundi-commits wants to merge 1 commit into
Conversation
The `batch_write` branch of `MySQLOnlineStore.online_write_batch` had two bugs that made it unusable: - It read `config.online_store.bacth_size` (typo), which raises `AttributeError` on the pydantic config, so enabling `batch_write` crashed before writing anything. - It serialized entity keys with `entity_key_serialization_version=2`, while the single-row write path and `online_read` in the same file use version 3. Rows written via the batch path therefore could not be read back. Fix the field name to `batch_size` and align the batch path on serialization version 3. Add unit tests (mocked connection, no live database) covering both. Note for a follow-up: unlike `postgres.py` / `sqlite.py`, this store hardcodes the serialization version instead of honoring `config.entity_key_serialization_version`; left as-is here to keep the change minimal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sht1BdriG5qHfkf5aF5gfc Signed-off-by: Karth <karthik.chundi@gmail.com>
shuchu
approved these changes
Sep 1, 2026
shuchu
self-requested a review
September 1, 2026 02:41
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6802 +/- ##
==========================================
+ Coverage 47.08% 47.13% +0.04%
==========================================
Files 419 419
Lines 51878 51878
Branches 7525 7525
==========================================
+ Hits 24429 24453 +24
+ Misses 25700 25672 -28
- Partials 1749 1753 +4
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
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
3 participants
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.
What this does
The
batch_writepath ofMySQLOnlineStore.online_write_batch(added in #5699) has two bugs that make it unusable:config.online_store.bacth_sizeis a typo forbatch_size.MySQLOnlineStoreConfigis aFeastConfigBaseModelwithextra="forbid", so:.bacth_sizeraisesAttributeError→ enablingbatch_write: truecrashes before a single row is written;BatchWriteMySQLOnlineStoreCreatorpasses"bacth_size": "1000"in its config dict, which failsMySQLOnlineStoreConfigvalidation withExtra inputs are not permitted.The batch path serializes entity keys with
entity_key_serialization_version=2, while the single-row write path andonline_readin the same file use version3. Rows written through the batch path are keyed differently from how they are read, so they cannot be read back. (Version < 3 is also discouraged perRepoConfig.)Changes
mysql.py:bacth_size→batch_size; batch pathentity_key_serialization_version2→3(matching the rest of the file).tests/universal/.../online_store/mysql.py:"bacth_size"→"batch_size"inBatchWriteMySQLOnlineStoreCreator.tests/unit/infra/online_store/test_mysql_online_store.py: two mocked-connection unit tests (no live DB) — one asserting the batch path readsbatch_sizewithout crashing, one asserting it serializes entity keys with version 3. Both fail onmasterand pass with this change.Testing
pytest sdk/python/tests/unit/infra/online_store/test_mysql_online_store.py sdk/python/tests/unit/infra/online_store/test_mysql_versioning.py→ 13 passed.ruff check/ruff format --checkclean;mypy feast/infra/online_stores/mysql_online_store/mysql.pyclean.Notes
Analysis and drafting were AI-assisted; I have reviewed and tested the change. Unlike
postgres.py/sqlite.py, this store hardcodes the serialization version rather than honoringconfig.entity_key_serialization_version— left as-is to keep this change minimal; happy to follow up.