[Cosmos] move system_key logic to query_iterable #22893
Conversation
simorenoh
requested review from
kushagraThapar,
simplynaveen20 and
xinlian12
as code owners
ghost
added
the
|
API changes have been detected in |
|
API changes have been detected in |
simorenoh
added
Client
|
@annatisch helped me out by spending time debugging the query code and seeing if it was possible to move the logic elsewhere within the query context in order to keep the logic based around this method. She found that it is possible, albeit not in the most efficient way since the method gets called every time for the first query's results on each iterator. I have tried doing some digging on my own as well and haven't been able to move past this in some way or another. I believe the way to move forward (for now) would be to make these inefficient changes that make the network calls with each iterator for the first query invoked by the user in order to unblock users in this month's release... and then work on getting a more efficient solution for the following release (since the async client is still in beta regardless). Would this be ok? @tjprescott @johanste |
|
This pull request is protected by Check Enforcer. What is Check Enforcer?Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass. Why am I getting this message?You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged. What should I do now?If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows: What if I am onboarding a new service?Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment: |
|
API changes have been detected in |
simorenoh
changed the title
…into new_metrics_advisor * 'main' of https://github.com/Azure/azure-sdk-for-python: (92 commits) [Mgmt Compute] Sanitize recordings (Azure#23331) [Tables] Release prep (Azure#23297) [Cosmos] move system_key logic to query_iterable (Azure#22893) [Cosmos] added additional warning to default consistency change (Azure#23225) update template (Azure#23333) [Storage] Address comments from API Review for March release (Azure#23294) code and test (Azure#23273) [textanalytics] release prep + fixing docstrings and links (Azure#23315) [Schema Registry] rename request kwargs to request options (Azure#23325) update new app service api version (Azure#23034) [SchemaRegistry] add tests for error codes (Azure#23140) Increment package version after release of azure-core (Azure#23319) Sync eng/common directory with azure-sdk-tools for PR 2824 (Azure#23314) Add python credscan in ci.yml (Azure#23307) [SchemaRegistry] avro api changes (Azure#23259) [KeyVualt KV] Fix cspell errors (Azure#23255) [KeyVault Certs] Fix cspell errors (Azure#23251) [AutoRelease] t2-containerregistry-2022-02-21-89653(Do not merge) (Azure#23155) [KeyVault Keys] Fix cspell errors (Azure#23250) Sync eng/common directory with azure-sdk-tools for PR 2830 (Azure#23267) ...
* moving system_key overhead to container creation for queries to work * Update CHANGELOG.md * Update CHANGELOG.md * Update database.py * moved awaiting logic to query_iterable_async.py * Update CHANGELOG.md * Update database.py
* moving system_key overhead to container creation for queries to work * Update CHANGELOG.md * Update CHANGELOG.md * Update database.py * moved awaiting logic to query_iterable_async.py * Update CHANGELOG.md * Update database.py
This is a problem that came up while we were having the big conversations around the async client in December, with the network call that is made through the
set_partition_key()method that was present in most container methods. It seems as though the changes with moving the await to the QueryIterableAsync class' aiter method did not end up working, and it has been reported by a customer that queries with partition keys didn't work, and I also verified myself (not sure how I tested this before that it didn't break, but here we are).With these changes we move the overhead of this call to the container initialization instead, since regardless any method a user would've first used with the container would have spent time on this network call regardless. This way, we also simplify the query logic in the back, and make sure that containers all have that check done when initialized.
Edit: This change is no longer what it previously described - although it was a good solution, it does not affect users who initialize their container proxies by calling
get_container_client(). Instead, the logic is now in the query_iterable class, and takes place during query execution.