Parse /_cat/shards output #1840
Conversation
| @@ -133,6 +134,16 @@ func (s Shards) GetShardsByNode() map[string]Shards { | |||
| return result | |||
| } | |||
|
|
|||
| // fixNodeNames extracts the name of the node from the output of the /_cat/shards API call | |||
| // see https://github.com/elastic/cloud-on-k8s/issues/1796 | |||
| func (s Shards) fixNodeNames() { | |||
There was a problem hiding this comment.
I find it a bit weird/dangerous to have this belong to and mutate the Shards object (which is not a pointer receiver).
We must not forget to call it otherwise the shards we get are not correct?
Another way of doing it is attaching our own UnmarshalJSON function to the Shards struct, which would get called automatically when parsing the API response. In that function we do call the default json unmarshallling function (giving us Shards with extra stuff in node names), and then patch the shards with the logic you used here. Related: http://choly.ca/post/go-json-marshalling/ to avoid the infinite JSON unmarshalling loop.
This would maybe be a bit safer (no need to think about "do I need to call fixNodeNames()?").
There was a problem hiding this comment.
Shards is a slice so the underlying array is updated even if it is not strictly a pointer receiver.
I didn't want to introduce a custom parser because I thought that at some point you may want to get the original data. I'll have a look to add it.
| "node": "" | ||
| }]` | ||
|
|
||
| func TestShards_fixNodeNames(t *testing.T) { |
There was a problem hiding this comment.
Maybe adapt the function name
| if err := json.Unmarshal(data, &aux); err != nil { | ||
| return err | ||
| } | ||
| for i, shard := range *aux { |
There was a problem hiding this comment.
Even though we have the link to the issue think I'd still appreciate a comment like:
// Strip extra information from the nodeName field (eg. "cluster-node-2 -> 10.56.2.33 8DqGuLtrSNyMfE2EfKNDgg" becomes "cluster-node-2").
|
For the records I have successfully run this PR against
|
Fix #1796
This PR also removes some unnecessary code from #1778