Skip to content

Add support for file paths for providing entity rows during batch retrieval #375

Merged
feast-ci-bot merged 1 commit intomasterfrom
batch-retrieval-file-path-dev
Dec 22, 2019
Merged

Add support for file paths for providing entity rows during batch retrieval #375
feast-ci-bot merged 1 commit intomasterfrom
batch-retrieval-file-path-dev

Conversation

@voonhous
Copy link
Copy Markdown
Collaborator

@voonhous voonhous commented Dec 18, 2019

This PR mirrors PR #365. The branch was deleted as i did not do a force-push but a delete-push to remote which resulted in PR#365 getting closed.

Description
Users should be able to provide large amounts of entity rows when retrieving batch features, but currently they are blocked by memory limits of pandas DataFrames.

Right now, for batch retrieval we already support Avro files as the format for sending entity rows, however this is only available on the Feast Serving API. The Python SDK hides this detail by doing

Entity_rows Pandas DF → .avro (local) → .avro (gcs) → BQ

This pull requests adds the ability for users to provide:

A pandas DataFrame with the "datetime" column
A local Avro file with the "event_timestamp" column
A gcs Avro file
A gcs wildcard path.
Examples:

entity_rows = [Pandas Dataframe]
entity_rows = subfolder/entities.avro
entity_rows = /data/subfolder/entities.avro
entity_rows = gs://food-recsys/folder/customer_entity_rows.avro
entity_rows = gs://food-recsys/folder/customer_entity_rows_*.avro
While datetime and event_timestamp are used interchangeably, there needs to be standardization within the SDK on which to use.

As of now:

datetime is enforced in Pandas DataFrame.
event_timestamp is enforced in local Avro file
No enforcement in files living in GCS. No validation will be done on GCS file paths.

Two additional tests have been ended to bq_batch_retrieval.py. The two tests test_get_batch_features_with_file and test_get_batch_features_with_gs_path will run an end to end test respectively for all changes mentioned above.

@voonhous
Copy link
Copy Markdown
Collaborator Author

/retest

@woop
Copy link
Copy Markdown
Member

woop commented Dec 22, 2019

/lgtm
/approved

woop
woop previously approved these changes Dec 22, 2019
@woop
Copy link
Copy Markdown
Member

woop commented Dec 22, 2019

/retest test-end-to-end

@woop
Copy link
Copy Markdown
Member

woop commented Dec 22, 2019

/retest

@woop
Copy link
Copy Markdown
Member

woop commented Dec 22, 2019

/lgtm
/approved

@voonhous
Copy link
Copy Markdown
Collaborator Author

/retest

@woop
Copy link
Copy Markdown
Member

woop commented Dec 22, 2019

/approved

@feast-ci-bot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: voonhous, woop

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot feast-ci-bot merged commit 30c2e0f into master Dec 22, 2019
@voonhous voonhous deleted the batch-retrieval-file-path-dev branch December 22, 2019 15:47
@ches ches mentioned this pull request May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants