[Cosmos] "Enable Content Response on Write" option and integration tests for items and query #1963
+837
−236
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.
This PR changes the default behavior for write operations to align with other Cosmos SDKs. By default, we instruct the server not to send the updated content of an item when writing it (Upsert, Replace, Patch, etc.). If a user wants the content returned for these operations, they must set the
enable_content_response_on_write
option. As part of this, I also changed the return types a bit:Response
instead ofResponse<T>
. In order forResponse<T>
to "work", theT
type has to implement thetypespec_client_core::Model
trait, a trait with is Azure/TypeSpec-specific. I didn't want to force users to do that. Fortunately, with Proposal: Rework body deserialization on Response<T> #1954 merged, we have another option. We can returnResponse
and users can callResponse::into_json_body
to parse the JSON body to their own custom type which need only implementserde::Deserialize
.Response
since we expect them to have no body by default. If a user setsenable_content_response_on_write
, they can parse the body with the sameResponse::into_json_body
method.query_...
methods which could return projections. This aligns the Go SDK, but users still have the ability to parse any response into their own type usingResponse::into_json_body
(even if the type isResponse<DatabaseProperties>
).In addition, I changed our doc examples to use
?
syntax (by having them be inasync fn doc() -> Result<(), Box<dyn std::error::Error>>
wrapper functions). It looks a lot cleaner that way, IMO, by removing all the noisy.unwrap()
calls and matching more closely to what we expect users to do in their own applications.Finally, I did need one change in the
azure_core_test
crate, because our live tests were broken by #1956 . I changedazure_core_test::recorded::start
to returnResult<Option<Session>>
(instead of justResult<Session>
). If the test is running in live mode, it'll returnOk(None)
, to indicate that no test proxy session was established. I think this is the right way to handle this, but I'm open to feedback.I'm not expecting action on this until the new year, because I know @heaths is out until the new year and the
azure_core_test
change will need his ✅ , but I wanted to open it up and get Cosmos folks looking at it.