Container properties cache consolidation #35731
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds a client level cache for container properties.
updated tests that were compensating for extra container reads, the extra reads no longer happen with adding the container cache properties in the client.
Fixed some reccomnded changes, and added optimizations for the cache. Only including needed properties info that will be unlikely to change.
Added a method in base.py to set the properties cache so we only have to change one location in case in the future we want to cache additional properties
Changed container cache to be a private attribute. Also added tests to verify the contents of the cache to be correct ones.
Co-authored-by: Kushagra Thapar <[email protected]>
Fixed broken tests that used old properties cache. Added samples to show how new properties cache works and best way to get the container properties
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
API change check API changes are not detected in this pull request. |
bambriz
requested review from
kushagraThapar,
simorenoh,
xinlian12 and
annatisch
as code owners
simorenoh
reviewed
May 23, 2024
simorenoh
reviewed
May 23, 2024
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR makes the cache for Container Properties (namely Partition Key Definition and RID) for a container to be cached in the client instance instead of the container instance.
Because at the moment the Python SDK does not handle caches the same as other SDKs which include refreshing the cache
(See Issue 35294) it is not a regression since it is just consolidating the cache of the properties to a single cache in the client instance and the python sdk has not yet implemented caching the container properties as the other SDKs do.
This PR will avoid a situation where: if you call single instances of a container multiple times from a client, it will perform a container read for every operation that does Create, Replace, Upsert, and ReadOffers. It will now instead use the cached container properties after the first container read has already been performed for that specific container.
The Client keeps a cache of properties of each container that is used in the lifetime of the client instance.
The current properties that are cached are "_self" which is the container link "dbsname/coll/containername" , the RID of the container, and the partition key definition of the container.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines