feat: DuckDB historical retrieval without entity dataframe#6108
feat: DuckDB historical retrieval without entity dataframe#6108Vperiodt wants to merge 12 commits into
Conversation
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@Vperiodt There is integration test tests/integration/offline_store/test_non_entity_mode.py, see if duckdb coverage can be included there |
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
|
@Vperiodt The lint checks are failing please check and rebase the PR. |
Sorry, something went wrong.
jyejare
left a comment
jyejare
left a comment
There was a problem hiding this comment.
Review Summary
The approach is clean — making entity_df optional and building a synthetic entity DataFrame from the feature view sources is a good way to enable date-range retrieval for DuckDB. Test coverage is solid (553 lines of unit tests + integration test updates).
However, CI is currently failing (lint-python and unit-test-python 3.10), so this is not merge-ready yet.
Blockers
- CI failures —
lint-pythonandunit-test-python (3.10, ubuntu-latest)are red on the latest run (Apr 13). Please investigate and fix. - Unconditional ibis import in
repo_configuration.py— breaks all tests whenibisis not installed. See inline comment for the fix.
Minor Items
- Missing docstring on
_build_entity_df_from_sources(flagged by @ntkathole, still pending). - Duplicate date-range logic — the start_date/end_date defaulting is copy-pasted across offline stores. Please extract to a shared utility or reuse
compute_non_entity_date_range.
Will ACK once CI is green and the blockers are addressed.
Sorry, something went wrong.
Vperiodt
force-pushed
the
patch-dataframe
branch
from
713e92e to
7eaff77
Compare
April 16, 2026 14:24
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Vperiodt
force-pushed
the
patch-dataframe
branch
from
f35adb9 to
da63740
Compare
April 17, 2026 11:43
Reviewers
4 participants
What this PR does / why we need it:
Adds date-range historical retrieval for the DuckDB offline store when entity_df is omitted.
Which issue(s) this PR fixes:
fixes #5832 related to #1611
Misc