-
Notifications
You must be signed in to change notification settings - Fork 3k
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] Clean up public surface area #40008
Conversation
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.
PR Overview
This PR cleans up the public API surface by removing unsupported parameters (such as session_token, etag, and match_condition) from various methods and by updating typehints and response hook usage. It also removes legacy functionality (e.g. lazy indexing) and converts synchronous test clean‐up calls to their asynchronous counterparts.
Reviewed Changes
File | Description |
---|---|
sdk/cosmos/azure-cosmos/azure/cosmos/aio/_database.py | Updates to remove session_token from method signatures and to add warnings for unsupported parameters in async methods. |
sdk/cosmos/azure-cosmos/azure/cosmos/cosmos_client.py | Removed unsupported parameters from database creation and query methods, replacing direct parameter usage with warnings. |
sdk/cosmos/azure-cosmos/azure/cosmos/database.py | Similar removal of unsupported parameters and updates to deprecation warnings in database and container creation methods. |
sdk/cosmos/azure-cosmos/azure/cosmos/documents.py | Removal of the Lazy indexing mode and improved docstring wording for indexing classes. |
sdk/cosmos/azure-cosmos/test/test_computed_properties_async.py | Updated container deletion calls to be asynchronous. |
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
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.
Left some minor comments, but LGTM
API change check APIView has identified API level changes in this PR and created following API reviews. |
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.
LGTM @simorenoh thanks for the changes.
We have recently found that some of the arguments and options that we expose in our public surface area are not relevant to several methods even though they are listed in their typehints. This PR aims at cleaning that up so that customers can be aware of the exact options that are available to them in every single one of our methods. Most of these changes revolved around the
session_token
,etag
,match_condition
andresponse_hook
arguments. Some notes on these:Session tokens only apply to item-level operations. Control plane operations are strongly consistent so this option has no relevance for those operations.
Etags and their relevant match conditions only apply to item-level operations, and within those only apply to replace and delete operations that deal with single items. Their typehints have been removed from all other instances where they were referenced.
Response hooks in the Python SDK will always take in two things: the response headers (dictionary of string -> string) and the response (dictionary of string -> any). Typehints have been updated to reflect this, and some wrong uses of response hooks were removed from query-type methods since calling the response hook at that point is broken behavior that does not in any way reference the results of the query and were using the wrong
last_response_headers
that would come from other operations at that point in the code.The following were cleaned up:
cosmos_client.py
session_token
option from thelist_databases
,query_databases
methods as they are not relevant to them.session_token
,etag
,match_condition
options from thecreate_database
,create_database_if_not_exists
,delete_database
methods as they are not relevant to them.database.py
session_token
option from theread
,list_containers
,query_containers
methods as they are not relevant to them.session_token
,etag
,match_condition
options from thecreate_container
,create_container_if_not_exists
,delete_container
,replace_container
methods as they are not relevant to them.container.py
session_token
option from theread
method as it is not relevant to this operation.etag
andmatch_condition
options from theupsert_item
,create_item
,execute_item_batch
,delete_all_items_by_partition_key
methods as they are not relevant to them.response_hook
typehints to be more accurate and removed their direct use before the queries get executed for theread_all_items
,query_items_change_feed
, andquery_items
methods.The same changes have been made to the async client's relevant files.
Also went ahead and fixed some tests I happened to see, as well as removing the mention of "Lazy" indexing since it has been unsupported for almost 5 years now.