etcdctl: fix migrate in outputing client.Node to json #6690
Conversation
|
cc @timothysc |
|
@wojtek-t FYI |
|
Only difference is the Defer to @xiang90 on backporting. |
|
@hongchaodeng Did this fix the migrate failure? |
|
@timothysc - what migration failure are you talking about? If there is a bug, this should be backported to 3.0 |
|
@wojtek-t yes we found a bug in yesterday on a large migrate we were testing. |
|
verified this fixes the migration issue we came across, please backport/cherry pick to 3.0 branch. |
|
/cc @smarterclayton |
| @@ -220,7 +220,7 @@ func writeKeys(w io.Writer, n *store.NodeExtern) uint64 { | |||
| if err != nil { | |||
| ExitWithError(ExitError, err) | |||
| } | |||
| fmt.Fprintf(w, string(b)) | |||
| fmt.Fprintln(w, string(b)) | |||
There was a problem hiding this comment.
@hongchaodeng can you explain why do we need to change fprintf to fprintln in the commit message in detail?
There was a problem hiding this comment.
I read through https://golang.org/src/fmt/print.go. If seems the only difference is that println adds a \n. do you have an example?
There was a problem hiding this comment.
Example for what?
There was a problem hiding this comment.
what is the problem. how fprintf and fprintln behaviors differently in this case.
There was a problem hiding this comment.
how fprintf and fprintln behaviors differently in this case.
printf("%") => %!(MISSING)
print("%") => %
Also this should be https://golang.org/pkg/fmt/#Fprint not https://golang.org/pkg/fmt/#Fprintln
Why print? Why not println?
There was a problem hiding this comment.
@hongchaodeng println adds a \n at the end of the line. why do we need that \n?
There was a problem hiding this comment.
println is easier for user to debug on the other end.
There was a problem hiding this comment.
no. it is not. it adds a char, which it should not. input and output should be the same.
There was a problem hiding this comment.
printf("%") => %!(MISSING)
print("%") => %
can you also add this into commit message?
Using printf will try to parse the string and replace special
characters. In migrate code, we want to just output the raw
json string of client.Node.
For example,
Printf("%\\") => %!\(MISSING)
Print("%\\") => %\
Thus, we should use print instead.
|
@xiang90 Fixed |
|
lgtm |
|
@mml - FYI @xiang90 @hongchaodeng - when can we expect backport of this to 3.0 branch? |
We should use print/println instead of printf.