Skip to content

Add support for DynamoDB and S3 registry#1483

Merged
feast-ci-bot merged 38 commits intofeast-dev:masterfrom
leonid133:feature/online_dynamodb
Jul 3, 2021
Merged

Add support for DynamoDB and S3 registry#1483
feast-ci-bot merged 38 commits intofeast-dev:masterfrom
leonid133:feature/online_dynamodb

Conversation

@leonid133
Copy link
Contributor

@leonid133 leonid133 commented Apr 21, 2021 •
edited by woop
Loading

What this PR does / why we need it:
We need to have an online serving backend with Dynamo to support customer deployments on AWS.

Which issue(s) this PR fixes:

Fixes #1409

Does this PR introduce a user-facing change?:
NONE

* Add support for S3 registry
* Add aws provider with Dynamo support

@feast-ci-bot
Copy link
Collaborator

feast-ci-bot commented Apr 21, 2021

Hi @leonid133. Thanks for your PR.

I'm waiting for a feast-dev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@woop
Copy link
Member

woop commented Apr 21, 2021

/ok-to-test

woop
woop reviewed Apr 21, 2021
View reviewed changes
woop
woop reviewed Apr 21, 2021
View reviewed changes
woop
woop reviewed Apr 21, 2021
View reviewed changes
woop
woop reviewed Apr 21, 2021
View reviewed changes
@woop
Copy link
Member

woop commented Apr 21, 2021

Thanks for this @leonid133!

@woop woop added the kind/feature New feature or request label Apr 21, 2021
@woop woop changed the title Feature/online dynamodb Add support for DynamoDB and S3 registry Apr 21, 2021
@woop
Copy link
Member

woop commented Apr 21, 2021

Also, we certainly should add the data model for Dynamo here: https://github.com/feast-dev/feast/tree/master/docs/specs

t0t07
t0t07 reviewed Apr 21, 2021
View reviewed changes
blvp
blvp reviewed Apr 21, 2021
View reviewed changes
@woop
Copy link
Member

woop commented Apr 22, 2021

Also, we will definitely need to have integrations tests with DynamoDB in order to have confidence in this code. We're happy to use our own AWS account, just let me know when you're ready the code and I can help set things up.

tedhtchang
tedhtchang reviewed Apr 22, 2021
View reviewed changes
@leonid133 leonid133 force-pushed the feature/online_dynamodb branch from d63be24 to 53284e8 Compare April 24, 2021 02:13
leonid133 added 10 commits June 28, 2021 14:46
@leonid133
The default value is None
15a787c 
Signed-off-by: lblokhin <[email protected]>
@leonid133
Remove Datastore from the docstring.
7eaa654 
Signed-off-by: lblokhin <[email protected]>
@leonid133
get rid of the return call from S3RegistryStore
1468117 
Signed-off-by: lblokhin <[email protected]>
@leonid133
merge two exceptions
5dbe429 
Signed-off-by: lblokhin <[email protected]>
@leonid133
For ci requirement
986d45e 
Signed-off-by: lblokhin <[email protected]>
@leonid133
remove configuration from test
79d85c7 
Signed-off-by: lblokhin <[email protected]>
@leonid133
feast-integration-tests for tests
f50b2fb 
Signed-off-by: lblokhin <[email protected]>
@leonid133
change test path
509c521 
Signed-off-by: lblokhin <[email protected]>
@leonid133
add fixture feature_store_with_s3_registry to test
cd67973 
Signed-off-by: lblokhin <[email protected]>
@leonid133
merge from master
5466d20 
Signed-off-by: lblokhin <[email protected]>
@leonid133 leonid133 force-pushed the feature/online_dynamodb branch 8 times, most recently from 685bd68 to 1ccfd7d Compare June 29, 2021 04:56
@leonid133
region required
3d1b78c 
Signed-off-by: lblokhin <[email protected]>
@leonid133 leonid133 force-pushed the feature/online_dynamodb branch from 1ccfd7d to 3d1b78c Compare June 29, 2021 05:35
@leonid133 leonid133 requested a review from tsotnet June 30, 2021 17:58
Tsotne Tabidze added 4 commits July 2, 2021 12:07
@feast-ci-bot
Copy link
Collaborator

feast-ci-bot commented Jul 3, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leonid133, tsotnet

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

@woop
Copy link
Member

woop commented Jul 4, 2021

Huge. Thanks @leonid133 @blvp @tsotnet !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oavdeev oavdeev oavdeev left review comments

@woop woop woop left review comments

@tedhtchang tedhtchang tedhtchang left review comments

@jklegar jklegar jklegar left review comments

@tsotnet tsotnet tsotnet approved these changes

@achals achals Awaiting requested review from achals

+3 more reviewers

@blvp blvp blvp left review comments

@t0t07 t0t07 t0t07 left review comments

@guykhazma guykhazma guykhazma left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@tsotnet tsotnet

Labels

approved kind/feature New feature or request lgtm ok-to-test release-note size/XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

DynamoDB support for online feature store