fix: Use configured timestamp column names in RemoteOnlineStore.online_write_batch#6596
fix: Use configured timestamp column names in RemoteOnlineStore.online_write_batch#6596franciscojavierarceo wants to merge 5 commits into
Conversation
|
@codex review please |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d8679cb7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| columnar_data: Dict[str, List[Any]] = defaultdict(list) | ||
|
|
||
| event_col = table.batch_source.timestamp_field or "event_timestamp" |
There was a problem hiding this comment.
When a remote FeatureStore.write_to_online_store() call targets an OnDemandFeatureView with write_to_online_store=True, PassthroughProvider.online_write_batch forwards that ODFV to this online store, and _convert_arrow_odfv_to_proto has already supplied artificial event/created timestamps. ODFVs do not define batch_source, so reading table.batch_source here raises AttributeError before the request is posted; the previous hard-coded timestamp-column path did not have this failure mode. Please fall back to the default columns when the table has no batch source.
Useful? React with 👍 / 👎.
…e_write_batch RemoteOnlineStore.online_write_batch hardcoded "event_timestamp" and "created" as column names when serializing data for the feature server. On the server side, _convert_arrow_fv_to_proto resolves columns by the feature view's configured batch_source.timestamp_field and batch_source.created_timestamp_column. When those differ from the hardcoded defaults (e.g. "created_at"), the server throws a KeyError. Use the feature view's configured column names instead, falling back to the previous defaults when unset. Closes #6595 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
30d0fdc to
a2768c6
Compare
The merge from master included #6599 (remove jsonschema, add attrs) which changed pyproject.toml without regenerating pixi.lock, causing pixi install --locked to fail in CI for duckdb-tests and ray-tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
|
|
||
| columnar_data: Dict[str, List[Any]] = defaultdict(list) | ||
|
|
||
| if table.batch_source is not None: |
There was a problem hiding this comment.
| if table.batch_source is not None: | |
| batch_source = getattr(table, 'batch_source', None) | |
| if batch_source is not None: |
2 participants
Summary
RemoteOnlineStore.online_write_batchhardcoded"event_timestamp"and"created"as column names when building the request body for the feature server. The server-side_convert_arrow_fv_to_protoresolves columns by the feature view's configuredbatch_source.timestamp_fieldandbatch_source.created_timestamp_column. When those differ from the hardcoded defaults (e.g."created_at"), the server throws aKeyError.created_timestamp_columnfallback.Closes #6595
Test plan
test_remote_online_store.pystill passTestRemoteOnlineStoreWriteBatchtests verify:event_timestamp,created) are used when configuredts,created_at) appear in the request body instead of hardcoded defaultscreated_timestamp_columnfalls back to"created"🤖 Generated with Claude Code