Skip to content
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

Patch struct causes errors when making requests #43

Closed
JoshMock opened this issue Jun 5, 2019 · 5 comments
Closed

Patch struct causes errors when making requests #43

JoshMock opened this issue Jun 5, 2019 · 5 comments

Comments

@JoshMock
Copy link

JoshMock commented Jun 5, 2019

I was trying to use the Patch struct to run patch_namespaced_ingress, but it appears based on both the documentation and the code that the only valid value of body is an empty struct reference &Patch{}.

When I do that, I get the following error at run time:

thread 'main' panicked at 'failed to patch ingress: ErrorMessage { msg: "Invalid method: PATCH" }

This is likely because the JSON body on the API request needs values in the form of a JSON merge patch, eg:

{
    "op" : "replace" ,
    "path" : "/some/path",
    "value" : "foo"
}

At first I thought this was me being new to Rust and just missing how to do pass the body parameter correctly, like maybe I needed to build a JSON body with Serde and cast it to a Patch struct. But then I showed my issue to another developer and they came to the same conclusion.

I'm using the v1_11 feature, for what it's worth. If there's any other information I can provide to clarify what I'm seeing, let me know! Thanks for building this!

@Arnavion
Copy link
Owner

Arnavion commented Jun 5, 2019

This was fixed in 79185b3 It'll be in 0.5.0

@Arnavion
Copy link
Owner

Arnavion commented Jun 5, 2019

As a workaround until then, you can take the http::Request<Vec<u8>> that you get from Ingress::patch_namespaced_ingress and replace its body with a body containing a serde_json::Value of your choice.

@JoshMock
Copy link
Author

JoshMock commented Jun 5, 2019

Thank you @Arnavion! I'll give this a shot.

@Arnavion
Copy link
Owner

Arnavion commented Jun 5, 2019

Note that you'll also need to set the Content-Type header to one of application/json-patch+json, application/merge-patch+json or application/strategic-merge-patch+json depending on what type of patch you're sending.


These are listed in the spec but I can't think of a way to easily surface this choice to the API user. Maybe something like

-struct Patch(serde_json::Value);
+enum Patch {
+    Json(serde_json::Value),
+    Merge(serde_json::Value),
+    StrategicMerge(serde_json::Value),
+}

and then have every API operation that takes a &Patch have special codegen.

It does look like patch operations are the only place where this kind of Content-Type choice matters, and it's always these three types, so it doesn't seem like a bad idea to have special codegen for it...

Arnavion pushed a commit that referenced this issue Jul 22, 2019
All operations with requests bodies, except for patch requests, now add
a `Content-Type: application/json` header to the request.

The `Patch` struct is now generated as an enum of `Json`, `Merge` and
`StrategicMerge` variants, each wrapping a `serde_json::Value`.

Patch operations use the variant of the `Patch` struct to set the appropriate
`Content-Type` header.

This commit also fixes the response types of delete-collection operations to
contain the list type of the associated type, not the associated type itself.

Ref #43
Fixes #49
Arnavion pushed a commit that referenced this issue Jul 22, 2019
All operations with requests bodies, except for patch requests, now add
a `Content-Type: application/json` header to the request.

The `Patch` type is now generated as an enum of `Json`, `Merge` and
`StrategicMerge` variants, each wrapping a `serde_json::Value`.

Patch operations use the variant of the `Patch` value to set the appropriate
`Content-Type` header.

This commit also fixes the response types of delete-collection operations to
contain the list type of the associated type, not the associated type itself.

Ref #43
Fixes #49
@Arnavion
Copy link
Owner

With 36334e6, Patch is now emitted as the enum I showed above, and all operations set the Content-Type header in the generated request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants