Sorry, something went wrong.
There was a problem hiding this comment.
This PR enables ingestion without requiring event timestamps by adding a disable_event_timestamp parameter throughout the materialization pipeline. When enabled, the current datetime is used instead of requiring explicit start and end timestamps.
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file| sdk/python/tests/foo_provider.py | Adds disable_event_timestamp parameter to test provider method |
| sdk/python/feast/infra/provider.py | Adds parameter to base provider interface with documentation |
| sdk/python/feast/infra/passthrough_provider.py | Implements parameter passing in passthrough provider |
| sdk/python/feast/infra/common/materialization_job.py | Adds parameter to MaterializationTask dataclass |
| sdk/python/feast/feature_store.py | Adds parameter to public materialize method |
| sdk/python/feast/cli/cli.py | Updates CLI command to support optional timestamps and new flag |
| infra/templates/README.md.jinja2 | Documents new materialization options |
| docs/reference/feast-cli-commands.md | Updates CLI command documentation |
| docs/getting-started/quickstart.md | Adds simple materialization example |
| docs/getting-started/concepts/data-ingestion.md | Documents timestamp-free option |
| README.md | Updates main README with new materialization options |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Sorry, something went wrong.
| feast materialize --disable-event-timestamp | ||
| ``` | ||
|
|
||
| The `--disable-event-timestamp` flag allows you to materialize all available feature data using the current datetime as the event timestamp, without needing to specify start and end timestamps. This is useful when your source data lacks proper event timestamp columns. |
There was a problem hiding this comment.
I think we do not need this flag, we can keep default end time as current time and start time as (current time - 24hr/a month) ?
Same for materialize-incremental command to have end date as current time by default.
Sorry, something went wrong.
There was a problem hiding this comment.
my concern is the significant change that has for users, right? making it optional would silently ingest data for users without telling them about it. i'd rather do more code work to make them opt in rather than have users accidentally ingest data.
Sorry, something went wrong.
There was a problem hiding this comment.
That's fair 👍
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good
Sorry, something went wrong.
What this PR does / why we need it:
This PR enables materialization without a timestamp by just using the current timestamp.
As updated in the documentation, this can be run with:
Which issue(s) this PR fixes:
Fixes #5624
Misc