dashboards: Add Yaxis.AutoMin, YAxis.AutoMax, and custom Unmarshal #167
Conversation
* Add in the AutoMin and AutoMax fields for the Yaxis struct so we can work around the times when datadog's API returns a string when we expect a float64. * Add a custom UnmarshalJSON for Yaxis to utilize the new AutoMin and AutoMax fields. related error: "json: cannot unmarshal string into Go struct field Yaxis.min of type float64"
|
This is in reference to issue #103 |
|
Thanks for the submission. Generally looks good. Can you run |
dashboards.go
Outdated
| return err | ||
| } | ||
|
|
||
| if *wrapper.Min == "auto" { |
There was a problem hiding this comment.
If the returned blob didn't contain the field, it would be null, so it's probably safer to check here.
if wrapper.Min != nil && *wrapper.Min == "auto" {
...
}There was a problem hiding this comment.
Added the additional nil check
dashboards.go
Outdated
| *y.AutoMin = true | ||
| y.Min = nil | ||
| } else { | ||
| *wrapper.AutoMin = false |
There was a problem hiding this comment.
The wrapper will go out of scope at this end of the func, so this isn't necessary.
There was a problem hiding this comment.
Just for clarification are you saying I do not need to set wrapper.AutoMin or the whole else block is not necessary?
There was a problem hiding this comment.
The block is necessary for parsing the float, but this line is unnecessary because wrapper.AutoMin will not be used again.
dashboards.go
Outdated
| } | ||
|
|
||
| if *wrapper.Min == "auto" { | ||
| *y.AutoMin = true |
There was a problem hiding this comment.
You can't dereference this here. You'll need to instantiate it first or don't use pointers for the Auto* fields, since they're local.
There was a problem hiding this comment.
removed pointers for the Auto* fields
* Convert Yaxis.AutoMin and Yaxis.AutoMax to `bool` rather than `*bool` since they are local fields
* Add json:"-" to `AutoMin` and `AutoMax` fields so the do not get marshalled
* Change the wrapper Min/Max fields to be type *json.Number so that Min/Max can take a string (“auto”) if necessary.
* I was running into an issue when {“min”: “auto”}, son.Unmarshal would complain `json: cannot unmarshal number into Go value of type string`
* Add Unit Tests for marshaling and unmarshalling Yaxis <-> JSON
dashboards.go
Outdated
| *y.AutoMin = true | ||
| y.Min = nil | ||
| } else { | ||
| *wrapper.AutoMin = false |
There was a problem hiding this comment.
The block is necessary for parsing the float, but this line is unnecessary because wrapper.AutoMin will not be used again.
ghost
merged commit
dashboards.go
Outdated
| AutoMax * bool | ||
| Scale *string `json:"scale,omitempty"` | ||
| Min *float64 `json:"min,omitempty"` | ||
| AutoMin bool `json:"-"` |
There was a problem hiding this comment.
Also FYI, making a field private will also prevent it from being marshalled (although it also prevents consumers from using it, obviously).
ghost
mentioned this pull request
related error: "json: cannot unmarshal string into Go struct field Yaxis.min of type
float64"
Followed the suggestions from @yfronto in the previous open PR.
Please let me know if this is the right direction and if you have anymore suggestions.