-
Notifications
You must be signed in to change notification settings - Fork 340
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
fix: save tags when using chunk/stream #4920
base: master
Are you sure you want to change the base?
Conversation
@@ -56,7 +56,7 @@ func (s *Service) chunkUploadStreamHandler(w http.ResponseWriter, r *http.Reques | |||
} | |||
|
|||
// if tag not specified use direct upload | |||
putter, err := s.newStamperPutter(r.Context(), putterOptions{ | |||
putter, err := s.newStamperPutter(context.Background(), putterOptions{ |
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.
why is context.Background() better 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.
probably because the go routine below handles the websocket connection but since this function returns, the context is canceled.
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.
yep
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.
pls add a comment here why this background context is necessary as it's not immediately obvious
Checklist
Description
Do not use HTTP request context so that tags will be saved when
putter.Done
is called after the HTTP connection is upgraded to websocket.Open API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
closes #4858
Screenshots (if appropriate):