-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[Cosmos] deprecate offer-named methods in favor of new throughput-named methods #22262
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
Conversation
* Removed some stuff * Looking at constructors * Updated request * Added client close * working client creation Co-authored-by: simorenoh <[email protected]>
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
|
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: |
|
It seems the non-standard indenting is still showing in the diff. |
tjprescott
left a comment
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.
So we still have a breaking change on account of wholesale renaming Offer to ThroughputProperties. @annatisch how do you recommend resolving that? Alias Offer to ThroughputProperties?
annatisch
left a comment
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 as @tjprescott mentioned - anyone who's written the following code should remain unbroken:
from azure.cosmos.offer import Offer
o = client.read_offer()
if isinstance(o, Offer):
print(o.properties)I'm pretty sure for the model itself this could just be aliased. However you'll probably need to keep the offer.py file around.
We're fortunate that this is a response type only (I can't see anywhere a customer would have constructed this themselves), so hopefully the number of people using this directly should be low.
annatisch
left a comment
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.
Looks good - probably just needs a changelog entry?
With these changes, we can officially mark read_offer() for deprecation and instantiate the more relevantly named read_throughput() method for database and container levels.
I went ahead and created a new ThroughputProperties object to replace the previously existing Offer object as well to match with the .NET and Java SDKs, although this bit is completely extra and could be reverted if we feel it's not neccessary.
I also know that the reasoning for this was the current state of documentation (which mostly references throughput as opposed to offers) but I found this too: https://docs.microsoft.com/en-us/rest/api/cosmos-db/offers - not sure if this is all there is, but wanted to double check on this point.