Add HEAD support for storage API object endpoint - #759
Merged
Conversation
Problem: As reporeted in fsouza#667. The storage API is replying 404 for existing objects when method HEAD is used, although GET replies with 200. Solution: Update storage API method route and handler to behave as "donwload" when HEAD method is used.
Problem: As a new user, one tends to simply use what comes in the README.md file without realizing there is plenty more features at hand. Solution: Update README.md to pointing out there is a -help flag that lists all the available configuration overrides. I was tempted to copy and paste its output, but it might add too much noise for the reader so disregarded that thought.
fsouza
approved these changes
May 10, 2022
fsouza
left a comment
fsouza
left a comment
Owner
There was a problem hiding this comment.
The staticcheck failure is already fixed on the main branch.
Thanks for contributing!
| func (s *Server) getObject(w http.ResponseWriter, r *http.Request) { | ||
| if alt := r.URL.Query().Get("alt"); alt == "media" { | ||
| if alt := r.URL.Query().Get("alt"); alt == "media" || r.Method == http.MethodHead { | ||
| s.downloadObject(w, r) |
Owner
There was a problem hiding this comment.
Oh I didn't realize this supported HEAD 🤔
We should probably rename this method, but doesn't need to be as part of this PR.
Contributor
Author
There was a problem hiding this comment.
Yeah, I had a similar thought, either renaming this function with another verb or splitting its functionality. However, I picked the route with less changes 😅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
I think I found a easy way to fix #667.
I also took the liberty to update the documentation, spelling out the
-helpflag usage for new users. Fun story, when I started using this emulator, I jumped into the code to figure out if there was a way to support my use case; I went from the endpoints to the handler implementation. Lastly, I found out all that the flags definitions were sitting inconfig.gofile, also that I could simply have used said flag all the time 😅.Both changes are in separted commits in case we want to drop or modify any of them.