◐ Shell
reader mode source ↗
Skip to content

feat: Add online_read_async for dynamodb#4244

Merged
franciscojavierarceo merged 16 commits into
feast-dev:masterfrom
robhowley:master
Jun 5, 2024
Merged

feat: Add online_read_async for dynamodb#4244
franciscojavierarceo merged 16 commits into
feast-dev:masterfrom
robhowley:master

Conversation

@robhowley

@robhowley robhowley commented May 31, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

  • adds aiobotocore to aws dependencies
  • adds online_read_async impl for dynamdb online reader
  • the use of the standard blocking boto client prevents the feast sdk from being able to handle real time web traffic
  • most of the diff is in the py3.X-[ci-]requirements.txt files

Which issue(s) this PR fixes:

Fixes

robhowley and others added 6 commits May 31, 2024 09:12
add aiobotocore and online_read_async to dynamo read …
bb0a4fb 
Signed-off-by: robhowley <rhowley@seatgeek.com>
format …
6b178bb 
Signed-off-by: robhowley <rhowley@seatgeek.com>
update test to run on dynamo …
a19d46d 
Signed-off-by: robhowley <rhowley@seatgeek.com>
duck type the table …
d7a982f 
Signed-off-by: robhowley <rhowley@seatgeek.com>
@robhowley
Merge pull request #5 from robhowley/rh-async-dynamo …
3db88cb 
feat: online_read_async for dynamodb
@robhowley
Merge branch 'feast-dev:master' into master
ee0b634
@robhowley robhowley changed the title feat: online_read_async for dynamodb May 31, 2024
@robhowley robhowley marked this pull request as draft May 31, 2024 13:36
update requirements files …
0217149 
Signed-off-by: robhowley <rhowley@seatgeek.com>
@robhowley robhowley marked this pull request as ready for review May 31, 2024 13:45
@robhowley

robhowley commented May 31, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

fyi @tokoko @breno-costa

robhowley added 2 commits May 31, 2024 11:18
fix: gotta await …
eeb3929 
Signed-off-by: robhowley <rhowley@seatgeek.com>
just pass full method, not client …
c4ba283 
Signed-off-by: robhowley <rhowley@seatgeek.com>
@robhowley robhowley marked this pull request as draft May 31, 2024 15:23
chore: allow for some duplication to make await cleaner/more obvious …
8c164c3 
Signed-off-by: robhowley <rhowley@seatgeek.com>
robhowley added 3 commits May 31, 2024 13:20
fix: client vs resource payloads …
a7488e3 
Signed-off-by: robhowley <rhowley@seatgeek.com>
fix resource vs client response handling …
a1d1ada 
Signed-off-by: robhowley <rhowley@seatgeek.com>
deserialize client response so that it looks like the table resource … …
6bd29f7 
…response

Signed-off-by: robhowley <rhowley@seatgeek.com>
@robhowley

robhowley commented May 31, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

unclear what the failure in pr-local-integration-tests / integration-test-python-local is all about since it's unrelated to my changes. it has flaked on me a few times, is there a way to just rerun it?

OSError: Generic S3 error: Error after 0 retries in 19.835973ms, max_retries:10, 
retry_timeout:180s, source:error sending request for url (
http://localhost:32798/test/2622baae-5bf8-42f0-b485-15b338730efd/integration_test_gh_run_9323007162_e7eafe_5258.customer_profile/_delta_log/_last_checkpoint): 
connection error: Connection reset by peer (os error 104)

@franciscojavierarceo im assuming you have perms for a rerun? 🙏

@robhowley robhowley marked this pull request as ready for review May 31, 2024 19:21
@breno-costa

breno-costa commented May 31, 2024

Copy link
Copy Markdown
Contributor

@robhowley I created a benchmark repo to test redis async implementation. If you want to make use of it to test your changes. You should be able to install your Feast version here and configure the dynamodb here.

@franciscojavierarceo

franciscojavierarceo commented Jun 1, 2024

Copy link
Copy Markdown
Member

I reran and it looks like a bucket issue failed. will have to dig into this more later. CC @lokeshrangineni @jeremyary @tokoko

@tokoko

tokoko commented Jun 2, 2024

Copy link
Copy Markdown
Collaborator

Definitely flaky, this just errored out other docs-only PR as well. I think we were missing this before because test-python-integration-local status code wasn't being reported properly due to docker info checking.

@robhowley Can you comment out this line here for now? I'll try to reenable it later...

temp disable due to flakiness in tests …
159259b 
Signed-off-by: robhowley <rhowley@seatgeek.com>
run linter …
fc9e603 
Signed-off-by: robhowley <rhowley@seatgeek.com>
relax to >2,<3 …
42e98d2 
Signed-off-by: robhowley <rhowley@seatgeek.com>
@robhowley robhowley requested a review from tokoko June 5, 2024 15:31
tokoko
tokoko approved these changes Jun 5, 2024
View reviewed changes

@tokoko tokoko left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

lgtm, thanks

@franciscojavierarceo franciscojavierarceo merged commit b5ef384 into feast-dev:master Jun 5, 2024
shuchu pushed a commit to shuchu/feast that referenced this pull request Jun 8, 2024
@robhowley @shuchu
feat: Add online_read_async for dynamodb (feast-dev#4244)
5a58a43
franciscojavierarceo pushed a commit that referenced this pull request Jun 18, 2024
@feast-ci-bot
chore(release): release 0.39.0 …
df46cae 
# [0.39.0](v0.38.0...v0.39.0) (2024-06-18)

### Bug Fixes

* Feast UI importlib change ([#4248](#4248)) ([5d486b8](5d486b8))
* Feature server no_feature_log argument error ([#4255](#4255)) ([15524ce](15524ce))
* Feature UI Server image won't start in an OpenShift cluster ([#4250](#4250)) ([4891f76](4891f76))
* Handles null values in data during GO Feature retrieval ([#4274](#4274)) ([c491e57](c491e57))
* Make Java gRPC client use timeouts as expected ([#4237](#4237)) ([f5a37c1](f5a37c1))
* Remove self assignment code line. ([#4238](#4238)) ([e514f66](e514f66))
* Set default values for feature_store.serve() function ([#4225](#4225)) ([fa74438](fa74438))

### Features

* Add online_read_async for dynamodb ([#4244](#4244)) ([b5ef384](b5ef384))
* Add the ability to list objects by `tags` ([#4246](#4246)) ([fbf92da](fbf92da))
* Added deadline to gRPC Java client ([#4217](#4217)) ([ff429c9](ff429c9))
* Adding vector search for sqlite ([#4176](#4176)) ([2478831](2478831))
* Change get_online_features signature, move online retrieval functions to utils ([#4278](#4278)) ([7287662](7287662))
* Feature/adding remote online store ([#4226](#4226)) ([9454d7c](9454d7c))
* List all feature views ([#4256](#4256)) ([36a574d](36a574d))
* Make RegistryServer writable ([#4231](#4231)) ([79e1143](79e1143))
* Remote offline Store  ([#4262](#4262)) ([28a3d24](28a3d24))
* Set optional full-scan for deletion ([#4189](#4189)) ([b9cadd5](b9cadd5))
@robhowley robhowley mentioned this pull request Oct 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@franciscojavierarceo franciscojavierarceo franciscojavierarceo approved these changes

@tokoko tokoko tokoko approved these changes

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@robhowley @breno-costa @franciscojavierarceo @tokoko