refactor(website): stub out go website endpoints#5681
Hidden character warning
Conversation
another-rex
left a comment
another-rex
left a comment
There was a problem hiding this comment.
LGTM, some minor comments
|
|
||
| var staticFS website.Config | ||
| if *staticDir != "" { | ||
| staticFS.StaticFS = os.DirFS(*staticDir) |
There was a problem hiding this comment.
hmm maybe use os.OpenRootFS? Another layer of protection against any path traversal stuff.
There was a problem hiding this comment.
I've added an embedded fs switch for prod.
This DirFS should only be used for local dev - I don't think path traversal is a big issue there.
| flag.Parse() | ||
|
|
||
| var staticFS website.Config | ||
| if *staticDir != "" { |
There was a problem hiding this comment.
Do we even need this check? I assume this should just not launch / panic if the flags are set to ""
There was a problem hiding this comment.
made this fail if no fs exists
| logger.ErrorContext(ctx, "Server error", slog.Any("error", err)) | ||
| return err | ||
| case <-ctx.Done(): | ||
| logger.InfoContext(ctx, "Shutting down website server...") |
There was a problem hiding this comment.
Is it fine to log with a ctx that's done?
There was a problem hiding this comment.
It is mostly okay (logging doesn't check if the context is done), but I've made this log on the shutdownCtx and used context.WithoutCancel there instead
2 participants
Set up all the endpoint stubs for the Go website migration.
Static files are to be
//go:embeded into a filesystem, which should make the website speedier (though the binary bigger).I haven't set up the datastore connections, nor dealt with rendering the Jinja templates in Go yet.