feat: DuckDB historical retrieval without entity dataframe#6108
feat: DuckDB historical retrieval without entity dataframe#6108Vperiodt wants to merge 7 commits intofeast-dev:masterfrom
Conversation
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
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: Vanshika Vanshika <vvanshik@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
| full_feature_names: bool = False, | ||
| **kwargs, | ||
| ) -> RetrievalJob: | ||
| start_date: Optional[datetime] = kwargs.get("start_date", None) |
There was a problem hiding this comment.
instead keep it as optional params in get_historical_features
| DEFAULT_ENTITY_DF_EVENT_TIMESTAMP_COL = "event_timestamp" | ||
|
|
||
|
|
||
| def _build_entity_df_from_sources( |
| ) | ||
|
|
||
|
|
||
| DEFAULT_ENTITY_DF_EVENT_TIMESTAMP_COL = "event_timestamp" |
There was a problem hiding this comment.
import this from from feast.infra.offline_stores.offline_utils import DEFAULT_ENTITY_DF_EVENT_TIMESTAMP_COL
|
@Vperiodt There is integration test tests/integration/offline_store/test_non_entity_mode.py, see if duckdb coverage can be included there |
| start_date = end_date - timedelta(seconds=max_ttl_seconds) | ||
| else: | ||
| start_date = end_date - timedelta(days=30) | ||
| start_date = make_tzaware(start_date) |
There was a problem hiding this comment.
Line 229 - Line 244 is common across the offline store to figure out the start_date & end_date. If its not too much, is it possible to create utility function which can be re-used?
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