-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Add online_read_async for dynamodb #4244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
bb0a4fb
add aiobotocore and online_read_async to dynamo read
6b178bb
format
a19d46d
update test to run on dynamo
d7a982f
duck type the table
3db88cb
Merge pull request #5 from robhowley/rh-async-dynamo
robhowley ee0b634
Merge branch 'feast-dev:master' into master
robhowley 0217149
update requirements files
eeb3929
fix: gotta await
c4ba283
just pass full method, not client
8c164c3
chore: allow for some duplication to make await cleaner/more obvious
a7488e3
fix: client vs resource payloads
a1d1ada
fix resource vs client response handling
6bd29f7
deserialize client response so that it looks like the table resource …
159259b
temp disable due to flakiness in tests
fc9e603
run linter
42e98d2
relax to >2,<3
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next
Next commit
add aiobotocore and online_read_async to dynamo read
Signed-off-by: robhowley <rhowley@seatgeek.com>
- Loading branch information
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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"] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the lower bound necessary here? maybe just put a |
||
|
|
||
| KUBERNETES_REQUIRED = ["kubernetes<=20.13.0"] | ||
|
|
||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not too sure about this, but is it a good idea to recreate client object on every call? Isn't there a performance penalty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the docs demonstrate and recommend usage within an async context manager. it is possible manually work with
_exit_stack.enter_async_contextat 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.