-
Notifications
You must be signed in to change notification settings - Fork 30
[BREAKING CHANGE] Support PATCH operations (partial updates) and bump @azure/cosmos to v3.17.3 #111
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
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
const patchOperations = [ | ||
{ op: "set", path: "/value", value: 200 }, | ||
{ op: "set", path: "/nested/property", value: "updated" }, | ||
{ op: "set", path: "/newProperty", value: "created" } | ||
]; | ||
|
||
await item.patch(patchOperations); | ||
const { resource: result } = await item.read(); |
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.
Testing is done using @azure/cosmos (v3.17.3) against the CosmosDB Server implementation. If the tests pass, it means the server implementation is compatible with the Cosmos SDK.
# The executed tests are for Cosmos SDK 4.5.0, but we only have partial support for 3.17.x. | ||
# That is why there are so many ignored test patterns. | ||
cd test/azure-sdk-for-js/sdk/cosmosdb/cosmos | ||
ACCOUNT_HOST="https://localhost:$port" npm run test:node:integration -- --testNamePattern='\^\(?!.*\(Authorization\|Change\ Feed\|Partition\|indexing\|Offer\ CRUD\|Permission\|Session\ Token\|sproc\|stored\ procedure\|Trigger\|TTL\|User\|Non\ Partitioned\|autoscale\|nonStreaming\|Iterator\|startFromBeginnin\|Full\ Text\ Search\|Full\ text\ search\ feature\|GROUP\ BY\|TOP\|DISTINCT\|ORDER\ BY\|LIMIT\|Conflicts\|readOffer\|validate\ trigger\ functionality\|SELECT\ VALUE\ AVG\ with\ ORDER\ BY\|changeFeedIterator\|test\ changefeed\|validate\ changefeed\ results\|New\ session\ token\|Validate\ SSL\ verification\|test\ batch\ operations\|test\ bulk\ operations\|test\ executeBulkOperations\|Id\ encoding\|Correlated\ Activity.*force\ query\ plan\|Correlated\ Activity.*GROUP\ BY\|aggregate\ query\ over\ null\ value\|Vector\ search\ feature\|Vector\ Search\ Query\|Bad\ partition\ key\ definition\|Reading\ items\ using\ container\|ClientSideEncryption\)\).*' |
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.
Because of yarn and other libraries bugs, I had to end up moving the test/azure-sdk-for-js
repository forward 3 years. This means:
- Lots of new tests cases we added. Now, we are testing against CosmosDB SDK 4.5.0. Because of that, I had to filter lots of new tests. Notice that the "patch" tests are not filtered, as we are interested in these running (new code we are adding in this PR).
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!
Changes
We are testing with
@azure/cosmos
v3.17.3 instead of v3.10.x. The reason for that is that we need v3.15.x to support Patch operations, and since we are already updating the library, I have bumped it to the latest 3.X version, which is maintenance mode (they are already in the version v4.5+).Bumping to version v4.X should be another PR as it is a breaking change and more risky. In this PR I am only interested in adding support for the Patch operations.
Another change is supporting Node 20 and 22 in the test matrix. We are also removing support for Node 14, 16 and 18, as they have reached their EOL and the new version from
@azure/cosmos
does not support that. Removing this support means it is a BREAKING CHANGE.The default NodeJS version has been moved from Node 14 to Node 20.
What is Patch updates?
Patch (Partial) updates allow us to only modify the fields that have changed from a CosmosDB document. Otherwise, the default behavior is to delete and create the Document again, which means the RU consumption increases in proportion to the Document size and we introduce more latency.
Testing
I have added integration tests to support the all patch operations, even if in the end we will only use three operations:
set
,remove
,replace
. The main reason is that the main tests from azure SDK try all patch operations, and we want to pass them.There is also an additional test to verify that the condition works, if used.
Also, I had to update the gitsubmodule used for testing to the latest one, due to errors and issues with all the previous versions.