-
Notifications
You must be signed in to change notification settings - Fork 350
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
routesrv: accept gzip encoding #2738
routesrv: accept gzip encoding #2738
Conversation
please create the PR on top of cw2023freeze |
1cb2383
to
a4f8037
Compare
a4f8037
to
ff88c27
Compare
ff88c27
to
63be56d
Compare
w.Header().Set(routing.RoutesCountName, strconv.Itoa(count)) | ||
|
||
if strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") && len(zdata) > 0 { | ||
w.Header().Set("Etag", `"`+hash+`+gzip"`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe use fmt.Sprintf?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I thought about it but it scatches me to use printf to add quotes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
w.Header().Set("Etag", `"`+hash+`"`)
w.Header().Set("Etag", `"`+hash+`+gzip"`)
// vs
w.Header().Set("Etag", fmt.Sprintf(`"%s"`, hash))
w.Header().Set("Etag", fmt.Sprintf(`"%s+gzip"`, hash))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if %q
makes it better
fmt.Sprintf("%q", hash)
fmt.Sprintf("%q", hash+"+gzip")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe: fmt.Sprintf("%qgzip", hash)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah no this does not work https://go.dev/play/p/xy_341u63TE.
I think @AlexanderYastrebov already tried the best one, in case he wants to have a string like:
"...gzip"
and not "..."gzip
Support gzip encoding to reduce response size. Signed-off-by: Alexander Yastrebov <[email protected]>
63be56d
to
7c387d1
Compare
👍 |
1 similar comment
👍 |
Support gzip encoding to reduce response size. Signed-off-by: Alexander Yastrebov <[email protected]>
Support gzip encoding to reduce response size. Signed-off-by: Alexander Yastrebov <[email protected]>
Support gzip encoding to reduce response size.