TrueCloudLab/frostfs-node
19
1
Fork 27
Code Issues 91 Pull requests 7 Projects Releases 1 Activity Actions

cli: Fix netinfo output for empty unknown parameters #1236

Closed
achuprov wants to merge 1 commit from achuprov/frostfs-node:feat/unknown_network_info_parameters into master
pull from: achuprov/frostfs-node:feat/unknown_network_info_parameters
merge into: TrueCloudLab:master
Conversation 0 Commits 1 Files changed 1 +6 -2
1 changed files with 6 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
cmd/frostfs-cli/modules/netmap/netinfo.go
View file

@ -50,8 +50,12 @@ var netInfoCmd = &cobra.Command{
cmd.Printf(format, "Maintenance mode allowed", netInfo.MaintenanceModeAllowed())
cmd.Println("FrostFS network configuration (other)")
netInfo.IterateRawNetworkParameters(func(name string, value []byte) {
cmd.Printf(format, name, hex.EncodeToString(value))
netInfo.IterateRawNetworkParameters(func(name string, rawValue []byte) {
fyrchik commented 2024-07-09 12:52:51 +00:00
Review
Copy link

I personally think the perfect behaviour was fine (we expect hex, we get empty hex)

I personally think the perfect behaviour was fine (we expect hex, we get empty hex)
achuprov commented 2024-07-09 13:10:05 +00:00
Review
Copy link

I think we should display the empty value (just like with namespace). What do you think about nil?

I think we should display the empty value (just like with namespace). What do you think about `nil`?
fyrchik commented 2024-07-09 13:20:49 +00:00
Review
Copy link

The same thing to me.
But if everyone agrees we need this, empty would be less Go-specific.

The same thing to me. But if everyone agrees we need this, `empty` would be less Go-specific.
achuprov commented 2024-07-15 07:29:32 +00:00
Review
Copy link

@aarifullin / @dstepanov-yadro / @elebedeva What do you think?

@aarifullin / @dstepanov-yadro / @elebedeva What do you think?
aarifullin commented 2024-07-15 08:29:37 +00:00
Review
Copy link

We should keep in mind that any cli output may be parsed (like test results).

value: ""

and

value: 57277d7b

As we can see these output results are not the same format. Either we need to frame non-nil output to quotes or omit it for the first case. I think if we keep it in current format - that's OK.

We should keep in mind that any cli output *may be* parsed (like test results). ``` value: "" ``` and ``` value: 57277d7b ``` As we can see these output results are not the same format. Either we need to *frame* non-nil output to quotes or omit it for the first case. I think if we keep it in current format - that's OK.
value := hex.EncodeToString(rawValue)
if value == "" {
value = `""`
}
cmd.Printf(format, name, value)
})
},
}