Conversation
| // is called directly, e.g.: | ||
| // uploadCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle") | ||
|
|
||
| uploadCmd.Flags().StringVar(&uploadBundlePath, "path", "supportbundle.tar.gz", "Path to the bundle that should be uploaded") |
There was a problem hiding this comment.
p, f, l, e ,c, d? Not sure if that is massively more usable
| jww.FEEDBACK.Println("Upload support bundle is not implemented...") | ||
| jww.FEEDBACK.Println("Uploading the provided support bundle") | ||
|
|
||
| contents, err := os.Open(uploadBundlePath) |
There was a problem hiding this comment.
Maybe after this is merged we can also add option to accept from stdin since a lot of ppl like to run things in docker containers these days
There was a problem hiding this comment.
Do you mean accept the support bundle contents from stdin?
| res, err := client.Do(req) | ||
| if err != nil { | ||
| return "", err | ||
| } |
| } | ||
|
|
||
| respData := responseData{} | ||
| json.Unmarshal(response, &respData) |
There was a problem hiding this comment.
err = json.Unmarshal(response, &respData)| return "", err | ||
| } | ||
|
|
||
| response, err := ioutil.ReadAll(res.Body) |
There was a problem hiding this comment.
how about json.NewDecoder(io.Reader)?
| } | ||
|
|
||
| if res.StatusCode != http.StatusCreated { | ||
| err = fmt.Errorf("Bad status when uploading support bundle: %s", res.Status) |
There was a problem hiding this comment.
do we use github.com/pkg/errors in this project? i prefer errors.Wrap
There was a problem hiding this comment.
Changed to use errors, but we're generating a new error anyways
| if err != nil { | ||
| return "", err | ||
| } | ||
| if _, err := io.Copy(formFile, file); err != nil { |
There was a problem hiding this comment.
there has got to be a way to stream this...
There was a problem hiding this comment.
There was a problem hiding this comment.
As far as I can tell, that's basically what we do...
Pipewriter instead of a buffered writer?
There was a problem hiding this comment.
Well here you load it all into memory and then stream upload. you probably need a goroutine somewhere
| req.Header.Set("Content-Type", w.FormDataContentType()) | ||
|
|
||
| // submit request | ||
| client := &http.Client{} |
| } | ||
|
|
||
| // closing the multipart writer sets the closing boundary | ||
| w.Close() |
There was a problem hiding this comment.
i think you need to close this if there is an error
| jww.FEEDBACK.Println("Uploading the provided support bundle") | ||
|
|
||
| contents, err := os.Open(uploadBundlePath) | ||
| defer contents.Close() |
There was a problem hiding this comment.
you may want this defer after the error
There was a problem hiding this comment.
I really wish github had a facepalm reaction.
| } | ||
|
|
||
| func streamingUploadFile(file *os.File, w *io.PipeWriter, data string, contentType chan string) { | ||
| defer file.Close() |
There was a problem hiding this comment.
i believe you close the file twice. you can remove this
| } | ||
| } | ||
|
|
||
| func streamingUploadFile(file *os.File, w *io.PipeWriter, data string, contentType chan string) { |
There was a problem hiding this comment.
how about just io.Reader and io.WriteCloser ?
There was a problem hiding this comment.
io.WriteCloser would work, but I do use file.Name. I don't think the api endpoint uses the filename at the moment and it could be omitted/be a placeholder, though the web interface does send it.
| writer := multipart.NewWriter(w) | ||
| defer writer.Close() | ||
|
|
||
| contentType <- writer.FormDataContentType() |
There was a problem hiding this comment.
can you just take this out of this function so you dont need the weird contentTypeChan
There was a problem hiding this comment.
I can, but then I have to pass both the pipewriter and the multipart.Writer so that close is called at the appropriate time for the pipewriter
Otherwise it just hangs. Unfortunately, it doesn't seem that multipart calls Close on the backing writer.
| // is called directly, e.g.: | ||
| // uploadCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle") | ||
|
|
||
| uploadCmd.Flags().StringVarP(&uploadBundlePath, "path", "p", "supportbundle.tar.gz", "Path to the bundle that should be uploaded") |
| go streamingUploadFile(file, write, string(bundleBytes), contentTypeChan) | ||
| multipartWriter := multipart.NewWriter(write) | ||
| contentType := multipartWriter.FormDataContentType() | ||
| go streamingUploadFile(file, write, multipartWriter, string(bundleBytes)) |
There was a problem hiding this comment.
go func() {
defer write.Close()
streamingUploadFile(file, multipartWriter, string(bundleBytes))
}()
laverya
force-pushed
the
andrewlavery/ch2986/command-line-support-bundle-upload
branch
from
e3ac63b to
8df789b
Compare
laverya
deleted the
andrewlavery/ch2986/command-line-support-bundle-upload
branch
2 participants
No description provided.