feat: Add online_read_async for dynamodb #4244

robhowley · 2024-05-31T13:28:04Z

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

Signed-off-by: robhowley <[email protected]>

feat: online_read_async for dynamodb

Signed-off-by: robhowley <[email protected]>

robhowley · 2024-05-31T13:39:57Z

sdk/python/tests/integration/online_store/test_universal_online.py

@@ -476,7 +476,7 @@ def test_online_retrieval_with_event_timestamps(environment, universal_data_sour


 @pytest.mark.integration
-@pytest.mark.universal_online_stores(only=["redis"])
+@pytest.mark.universal_online_stores(only=["redis", "dynamodb"])


add dynamo to the async online store tests

robhowley · 2024-05-31T13:48:35Z

fyi @tokoko @breno-costa

Signed-off-by: robhowley <[email protected]>

…response Signed-off-by: robhowley <[email protected]>

tokoko · 2024-05-31T19:09:20Z

sdk/python/requirements/py3.10-requirements.txt

@@ -20,32 +20,38 @@ charset-normalizer==3.3.2
    # via requests
 click==8.1.7
    # via
+    #   feast (setup.py)


just curious, what command did you run to lock these files? I'm not sure why this feast (setup.py) lines are being added here...

make lock-python-dependencies PYTHON=3.xx

robhowley · 2024-05-31T19:20:23Z

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 · 2024-05-31T19:27:03Z

sdk/python/feast/infra/online_stores/dynamodb.py

+        def to_tbl_resp(raw_client_response):
+            return {
+                "entity_id": deserialize(raw_client_response["entity_id"]),
+                "event_ts": deserialize(raw_client_response["event_ts"]),
+                "values": deserialize(raw_client_response["values"]),
+            }


the dynamo client returns stuff in the form

{ "field_name": {"TYPE": "value"}}

as opposed to the resource which just returns

{ "field_name": value}

dynamodb has a deserializer utility to do the conversion

breno-costa · 2024-05-31T20:12:03Z

@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 · 2024-06-01T18:34:37Z

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

tokoko · 2024-06-01T19:58:59Z

sdk/python/feast/infra/online_stores/dynamodb.py

+        return self._aioboto_session
+
+    def _get_aiodynamodb_client(self, region: str):
+        return self._get_aioboto_session().create_client("dynamodb", region_name=region)


Not too sure about this, but is it a good idea to recreate client object on every call? Isn't there a performance penalty?

the docs demonstrate and recommend usage within an async context manager. it is possible manually work with _exit_stack.enter_async_context at app startup/shutdown but wanted to avoid using protected methods and adding app complexity without first knowing it was really needed. seemed like an "as needed follow up" type of investigation.

tokoko · 2024-06-02T06:50:28Z

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...

Signed-off-by: robhowley <[email protected]>

tokoko · 2024-06-03T16:16:53Z

setup.py

@@ -84,7 +84,7 @@
    "hiredis>=2.0.0,<3",
 ]

-AWS_REQUIRED = ["boto3>=1.17.0,<2", "docker>=5.0.2", "fsspec<=2024.1.0"]
+AWS_REQUIRED = ["boto3>=1.17.0,<2", "docker>=5.0.2", "fsspec<=2024.1.0", "aiobotocore>=2.13.0"]


Is the lower bound necessary here? maybe just put a >2,<3 unless there's some specific reason

Signed-off-by: robhowley <[email protected]>

tokoko

lgtm, thanks

# [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 and others added 6 commits May 31, 2024 09:12

add aiobotocore and online_read_async to dynamo read

bb0a4fb

Signed-off-by: robhowley <[email protected]>

format

6b178bb

Signed-off-by: robhowley <[email protected]>

update test to run on dynamo

a19d46d

Signed-off-by: robhowley <[email protected]>

duck type the table

d7a982f

Signed-off-by: robhowley <[email protected]>

Merge pull request #5 from robhowley/rh-async-dynamo

3db88cb

feat: online_read_async for dynamodb

Merge branch 'feast-dev:master' into master

ee0b634

robhowley changed the title ~~feat: online_read_async for dynamodb~~ feat: Add online_read_async for dynamodb May 31, 2024

robhowley marked this pull request as draft May 31, 2024 13:36

update requirements files

0217149

Signed-off-by: robhowley <[email protected]>

robhowley commented May 31, 2024

View reviewed changes

robhowley marked this pull request as ready for review May 31, 2024 13:45

franciscojavierarceo added the ok-to-test label May 31, 2024

robhowley added 2 commits May 31, 2024 11:18

fix: gotta await

eeb3929

Signed-off-by: robhowley <[email protected]>

just pass full method, not client

c4ba283

Signed-off-by: robhowley <[email protected]>

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 <[email protected]>

robhowley force-pushed the master branch from c20b79f to 8c164c3 Compare May 31, 2024 16:36

robhowley added 3 commits May 31, 2024 13:20

fix: client vs resource payloads

a7488e3

Signed-off-by: robhowley <[email protected]>

fix resource vs client response handling

a1d1ada

Signed-off-by: robhowley <[email protected]>

deserialize client response so that it looks like the table resource …

6bd29f7

…response Signed-off-by: robhowley <[email protected]>

tokoko reviewed May 31, 2024

View reviewed changes

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

robhowley commented May 31, 2024

View reviewed changes

tokoko reviewed Jun 1, 2024

View reviewed changes

temp disable due to flakiness in tests

159259b

Signed-off-by: robhowley <[email protected]>

run linter

fc9e603

Signed-off-by: robhowley <[email protected]>

robhowley requested review from franciscojavierarceo and tokoko June 3, 2024 12:55

tokoko reviewed Jun 3, 2024

View reviewed changes

relax to >2,<3

42e98d2

Signed-off-by: robhowley <[email protected]>

robhowley requested a review from tokoko June 5, 2024 15:31

tokoko approved these changes Jun 5, 2024

View reviewed changes

franciscojavierarceo approved these changes Jun 5, 2024

View reviewed changes

franciscojavierarceo merged commit b5ef384 into feast-dev:master Jun 5, 2024
16 checks passed

shuchu pushed a commit to shuchu/feast that referenced this pull request Jun 8, 2024

feat: Add online_read_async for dynamodb (feast-dev#4244)

5a58a43

robhowley mentioned this pull request Oct 17, 2024

perf: Add init and cleanup of long lived resources #4642

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add online_read_async for dynamodb #4244

feat: Add online_read_async for dynamodb #4244

robhowley commented May 31, 2024 •

edited

Loading

robhowley May 31, 2024

robhowley commented May 31, 2024 •

edited

Loading

tokoko May 31, 2024

robhowley May 31, 2024

franciscojavierarceo Jun 1, 2024

robhowley commented May 31, 2024 •

edited

Loading

robhowley May 31, 2024

breno-costa commented May 31, 2024

franciscojavierarceo commented Jun 1, 2024

tokoko Jun 1, 2024

robhowley Jun 3, 2024

tokoko commented Jun 2, 2024

tokoko Jun 3, 2024

tokoko left a comment

feat: Add online_read_async for dynamodb #4244

feat: Add online_read_async for dynamodb #4244

Conversation

robhowley commented May 31, 2024 • edited Loading

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes

Choose a reason for hiding this comment

robhowley commented May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robhowley commented May 31, 2024 • edited Loading

Choose a reason for hiding this comment

breno-costa commented May 31, 2024

franciscojavierarceo commented Jun 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tokoko commented Jun 2, 2024

Choose a reason for hiding this comment

tokoko left a comment

Choose a reason for hiding this comment

robhowley commented May 31, 2024 •

edited

Loading

robhowley commented May 31, 2024 •

edited

Loading

robhowley commented May 31, 2024 •

edited

Loading