Fix header handling for resource create requests #56
Conversation
| * or matches any, JSON will be used. | ||
| */ | ||
| public String selectHeaderContentType(String[] contentTypes) { | ||
| if (contentTypes.length == 0) { |
There was a problem hiding this comment.
I don't think we can do this, we'll need to make this change in the upstream swagger-codegen and I don't think we can land it there.
The right thing to do is to fix the Kubernetes swagger to not have this.
There was a problem hiding this comment.
... and I don't think we can land it there.
We did this for python client. swagger-api/swagger-codegen#4152
I think it is a reasonable thing to do because swagger client prefers json and when content type is */*, that includes json.
The right thing to do is to fix the Kubernetes swagger to not have this.
I also agree with this regardless of swagger-codegen fix.
There was a problem hiding this comment.
My read of the Kubernetes swagger was that it was declaring that it, as a server, accepts anything (*/*). If this is true, it's up to the client to decide what it wants to send, and requires either swagger-codegen to be smarter about assigning content types or us as consumers of the generated code to be explicit in what we are sending. But of course, if Kubernetes only accepts json & yaml for example then the right fix is in the Kubernetes swagger.
An alternative to modifying ApiClient in place is to subclass it and override this method to avoid having our changes overwritten by running codegen. Although - when I tested this against the autoupdate script it left my changes in place, so I'm not sure what's going on there.
There was a problem hiding this comment.
Ok, let's due the PR in upstream swagger-codegen. They've been great about quick turnaround in the past. Once that is merged, I'm happy to merge this PR.
There was a problem hiding this comment.
(and let me know if you need help with the PR, I've done a bunch of them myself)
There was a problem hiding this comment.
Sounds good.
And thanks - I've done one myself now too so this shouldn't be too bad.
|
Closing for: #58 |
3 participants
A specific content type needs to be specified for create requests; the kubernetes swagger.json lists
consumes [ "*/*" ]for most resource create requests, which can't be set as a content-type header directly.Since all requests are serialized into JSON this seems appropriate.
@brendandburns @mbohlool PTAL