-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[Cosmos] CosmosDB asynchronous client #21404
Conversation
* Removed some stuff * Looking at constructors * Updated request * Added client close * working client creation Co-authored-by: simorenoh <simonmorenohe@gmail.com>
database read works, but ignored exception is returned: Fatal error on SSL transport NoneType has no attribute 'send' (_loop._proactor.send) RuntimeError: Event loop is closed Unclosed connector/ connection
Added methods needed to use async with when initializing client, but logs output "Exception ignored... Runtime Error: Event loop is closed"
Move branch into local fork
missing upsert and other resources
missing read_all_items and both query methods for container class
Complete item CRUD functionality - only missing queries
Query functionality and container query methods
also fixed README and added examples_async
sdk/cosmos/azure-cosmos/README.md
Outdated
url = os.environ['ACCOUNT_URI'] | ||
key = os.environ['ACCOUNT_KEY'] | ||
database_name = 'testDatabase' | ||
container_name = 'products' |
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.
ditto. I was a bit confused when I saw these being referred in the function below without realizing they are constants.
'productName': 'Widget', | ||
'productModel': 'Model {0}'.format(i) | ||
} | ||
) |
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.
Do we need to close the client here too?
await client.close()
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.
this is that second way to initialize I had mentioned above - the way it is done here, it gets disposed once the with
statement gets exited
sdk/cosmos/azure-cosmos/README.md
Outdated
|
||
### Queries with the asynchronous client | ||
|
||
Queries work the same way for the most part, with one exception being the absence of the `enable_cross_partition` flag in the request; queries without a specified partition key value will now by default atempt to do a cross partition query. Results can be directly iterated on, but because queries made by the asynchronous client return AsyncIterable objects, results can't be cast into lists directly; instead, if you need to create lists from your results, use Python's list comprehension to populate a list: |
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.
Queries work the same way for the most part, with one exception being the absence of the `enable_cross_partition` flag in the request; queries without a specified partition key value will now by default atempt to do a cross partition query. Results can be directly iterated on, but because queries made by the asynchronous client return AsyncIterable objects, results can't be cast into lists directly; instead, if you need to create lists from your results, use Python's list comprehension to populate a list: | |
Queries work the same way for the most part, with one exception being the absence of the `enable_cross_partition` flag in the request; queries without a specified partition key value will now by default attempt to do a cross partition query. Results can be directly iterated on, but because queries made by the asynchronous client return AsyncIterable objects, results can't be cast into lists directly; instead, if you need to create lists from your results, use Python's list comprehension to populate a list: |
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.
- Is this the reason that we don't have an await when querying? Should we make that more explicit?
- Why do we need the word "async" again in the function
- IIRC point queries operate differently. Should we add an example and explain the differentiation?
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.
- Yeah this is that point I had brought up with you before, I feel like I tried to explain it but I'm not sure it's completely understandable... let's talk on this to reach a conclusion.
- We need the word async in the function definition not because of the query, but because we do an
async for
loop right after - any function that has eitherawait
orasync
within has to be defined as async by python - This is the case even for the normal sync client, and I believe should be documented in the actual Cosmos docs (not just SDK), found this for instance: https://devblogs.microsoft.com/cosmosdb/point-reads-versus-queries/
sdk/cosmos/azure-cosmos/README.md
Outdated
from azure.cosmos.aio import CosmosClient | ||
import os | ||
|
||
url = os.environ['ACCOUNT_URI'] |
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.
ditto on the constants
container_name = 'products' | ||
container = database.get_container_client(container_name) | ||
|
||
async def create_lists(): |
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.
Do we need to close the client in this example too? I'm not seeing "with" being used either.
Clarifying comments for README Co-authored-by: Gahl Levy <75269480+gahl-levy@users.noreply.github.com>
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.
One small change.
Co-authored-by: Simon Moreno <30335873+simorenoh@users.noreply.github.com>
* initial commit * Client Constructor (Azure#20310) * Removed some stuff * Looking at constructors * Updated request * Added client close * working client creation Co-authored-by: simorenoh <simonmorenohe@gmail.com> * read database database read works, but ignored exception is returned: Fatal error on SSL transport NoneType has no attribute 'send' (_loop._proactor.send) RuntimeError: Event loop is closed Unclosed connector/ connection * Update simon_testfile.py * with coroutine Added methods needed to use async with when initializing client, but logs output "Exception ignored... Runtime Error: Event loop is closed" * Update simon_testfile.py * small changes * async with returns no exceptions * async read container * async item read * cleaning up * create item/ database methods * item delete working * docs replace functionality missing upsert and other resources * upsert functionality missing read_all_items and both query methods for container class * missing query methods * CRUD for udf, sproc, triggers * initial query logic + container methods * missing some execution logic and tests * oops * fully working queries * small fix to query_items() also fixed README and added examples_async * Update _cosmos_client_connection_async.py * Update _cosmos_client_connection.py * documentation update * updated MIT dates and get_user_client() description * Update CHANGELOG.md * Delete simon_testfile.py * leftover retry utility * Update README.md * docs and removed six package * changes based on comments still missing discussion resolution on SSL verification and tests for async functionality under test module (apart from samples which are basically end to end tests) * small change in type hints * updated readme * fixes based on conversations * added missing type comments * update changelog for ci pipeline * added typehints, moved params into keywords, added decorators, made _connection_policy private * changes based on sync with central sdk * remove is_system_key from scripts (only used in execute_sproc) is_system_key verifies that an empty partition key is properly dealt with if ['partitionKey']['systemKey'] exists in the container options - however, we do not allow containers to be created with empty partition key values in the python sdk, so the functionality is needless * Revert "remove is_system_key from scripts (only used in execute_sproc)" Reverting last commit, will find way to init is_system_key for now * async script proxy using composition * pylint * capitalized constants * Apply suggestions from code review Clarifying comments for README Co-authored-by: Gahl Levy <75269480+gahl-levy@users.noreply.github.com> * closing python code snippet * last doc updates * Update sdk/cosmos/azure-cosmos/CHANGELOG.md Co-authored-by: Simon Moreno <30335873+simorenoh@users.noreply.github.com> Co-authored-by: annatisch <antisch@microsoft.com> Co-authored-by: Gahl Levy <75269480+gahl-levy@users.noreply.github.com> Co-authored-by: Travis Prescott <tjprescott@users.noreply.github.com>
This PR contains the changes for the working language-native asynchronous client for cosmosdb. The async samples that were added showcase how the different functions can be used with async/await, and serve to show end to end functionality of each resource.
Any and all feedback is appreciated!