-
Notifications
You must be signed in to change notification settings - Fork 360
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
UI Etag support for embedded content #7132
Conversation
Scan embedded content on startup and serve etag header to FileServer. FileServer will use the Etag and provide no content change if needed. This will allow cache all static resources, which allow better performance after the first load, in the web ui.
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.
Minor comments
pkg/api/ui_handler.go
Outdated
@@ -34,7 +33,7 @@ func NewUIHandler(gatewayDomains []string, snippets []params.CodeSnippet) http.H | |||
panic(err) | |||
} | |||
fileSystem := http.FS(injectedContent) | |||
nocacheContent := middleware.NoCache(http.StripPrefix("/", http.FileServer(fileSystem))) | |||
nocacheContent := EtagMiddleware(injectedContent, http.StripPrefix("/", http.FileServer(fileSystem))) |
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.
Modify variable name
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.
Thanks - I missed that
pkg/api/etag_middleware.go
Outdated
} | ||
defer func() { _ = f.Close() }() | ||
|
||
h := md5.New() |
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.
// #nosec
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.
Checking if crc32 will do a better work here
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.
keeping the md5
pkg/api/etag_middleware.go
Outdated
package api | ||
|
||
import ( | ||
"crypto/md5" |
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.
// #nosec
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.
LGTM!
Scan embedded content on startup and serve etag header to FileServer. FileServer will use the Etag and provide no content change if needed.
This will allow cache all static resources, which allow better performance after the first load, in the web ui.
Close #7131