Skip to content
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] Improve system key computation logic #23821

Merged
merged 81 commits into from
Apr 6, 2022
Merged

[Cosmos] Improve system key computation logic #23821

merged 81 commits into from
Apr 6, 2022

Conversation

simorenoh
Copy link
Member

@simorenoh simorenoh commented Apr 5, 2022

Follow up to #22893
Previously, we were using this call to set the partition key value for a query request if it was passed in:
image

This awaitable being passed would then be resolved by the _query_iterable_async class once for every query being executed with a partition key, which is not ideal.

This small change in the function's return value makes it so we only await/do the network call if the user passed in a NonePartitionKeyValue, effectively solving the issue for all other queries and 99% of user cases.

Fixes #23169

simorenoh and others added 30 commits August 13, 2021 13:14

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
* Removed some stuff

* Looking at constructors

* Updated request

* Added client close

* working client creation

Co-authored-by: simorenoh <simonmorenohe@gmail.com>

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
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"
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
@azure-sdk
Copy link
Collaborator

API change check for azure-cosmos

API changes have been detected in azure-cosmos. You can review API changes here

Copy link
Member

@xinlian12 xinlian12 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@simorenoh simorenoh merged commit 21e367a into Azure:main Apr 6, 2022
@simorenoh simorenoh deleted the improve-system-key-logic branch April 6, 2022 16:26
rakshith91 pushed a commit to rakshith91/azure-sdk-for-python that referenced this pull request Apr 7, 2022
* 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>

* version update

* cosmos updates for release

* fix connection string comma

* Update CHANGELOG.md

* fixing extra await keyword in sample

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update cosmos_client.py

* Update container.py

* Update container.py

* changelog

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>
rakshith91 pushed a commit to rakshith91/azure-sdk-for-python that referenced this pull request Apr 10, 2022
* 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>

* version update

* cosmos updates for release

* fix connection string comma

* Update CHANGELOG.md

* fixing extra await keyword in sample

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update cosmos_client.py

* Update container.py

* Update container.py

* changelog

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Cosmos] move is_system_key logic to be computed only once on container usage
6 participants