这是indexloc提供的服务,不要输入任何密码
Skip to content

Adding Content-Type headers to POST and PATCH requests #23

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

Merged
merged 4 commits into from
Apr 25, 2022

Conversation

ctgowrie
Copy link
Contributor

@ctgowrie ctgowrie commented Apr 25, 2022

While working on some API improvements, I noticed that requests from the terraform provider do not set the content-type header, so this adds Content-Type to all POST and PATCH requests where relevant.

  • sets default content-type header as application/json for any doRequest calls with a body
  • change to nil body for GET and DELETE requests (so we do not set content-type header)
  • application/octet-stream for file upload requests

@ctgowrie ctgowrie requested a review from dglsparsons as a code owner April 25, 2022 14:55
@dglsparsons
Copy link
Collaborator

Hey @ctgowrie. Thanks for this - look like a nice simple improvement (and well spotted).
I wonder if we need a helper function so this is more reliable / less copy-paste code. I'll merge this in the meantime though, as it's a nice improvement.

@ctgowrie
Copy link
Contributor Author

@dglsparsons Thanks! I can move the application/json header setting into doRequest if req.Body != nil && req.Header.Get("Content-Type") == "" pretty easily if you'd prefer

@dglsparsons
Copy link
Collaborator

@ctgowrie That sounds like a good plan! I imagine pretty much every request is JSON with the sole exception of the file uploads.

@dglsparsons dglsparsons merged commit ec7a2c1 into main Apr 25, 2022
@dglsparsons dglsparsons deleted the request-content-types branch April 25, 2022 18:18
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

Successfully merging this pull request may close these issues.

2 participants