Move create document to pipeline architecture #331
Conversation
|
I love it ❤️. Before going further, would you please try and use the just merged authorization policy? It should be enough to call PS: I'll make sure to run the E2E tests on my account as soon as the integration tests will be green. |
rylev
force-pushed
the
create-document-pipeline
branch
from
635f022 to
1946f4b
Compare
There was a problem hiding this comment.
Nice work. 👍
Unfortunately something is amiss because the generated requests aren't compliant with the specs (and as such they fail).
I believe we should accept structs that implement both CosmosEntity and Serialize so we can easily extract the correct partition key.
rylev
force-pushed
the
create-document-pipeline
branch
from
1946f4b to
b77b98b
Compare
|
@MindFlavor thanks for the review. I believe I've addressed all the issues you found. Can you review again? Thanks! |
There was a problem hiding this comment.
Just a quick URI to fix and the tests pass!
As a side note, the permission_token_usage test fails but it's not due to your code. It's a regression introduced by the AuthorizationPolicy (or, better, by the fact that some methods use it and some don't). I will open a issue to tackle it in another PR.
|
|
||
| fn prepare_request_pipeline(&self, http_method: http::Method) -> Request { | ||
| let path = &format!( | ||
| "dbs/{}/colls/{}", |
There was a problem hiding this comment.
This should be "dbs/{}/colls/{}/docs" as per https://docs.microsoft.com/en-us/rest/api/cosmos-db/create-a-document
Continuing #290.
IMPORTANT: I've not yet tested these changes, so while it's ready for a review, I'd like to run the e2e tests before merging.