CLOUDP-82836: Parse service version #248
Conversation
mongodbatlas/mongodbatlas.go
Outdated
| } | ||
|
|
||
| // GetServiceVersion parses version information returned in the response. | ||
| func (resp *Response) GetServiceVersion() *ServiceVersion { |
There was a problem hiding this comment.
[nit] getters are non idiomatic on go
| func (resp *Response) GetServiceVersion() *ServiceVersion { | |
| func (resp *Response) ServiceVersion() *ServiceVersion { |
There was a problem hiding this comment.
I know it does not sound right but I did not think Get as a getter here 😄
| result.GitHash = keyvalue[1] | ||
| case "versionString": | ||
| result.Version = keyvalue[1] | ||
| } |
There was a problem hiding this comment.
[q] Should we log/fail on any other case? I generally don't like switches without a default, as changes can go undetected. If we had a default error, along with a unit test, a structure change to the client would immediately fail here.
There was a problem hiding this comment.
Thanks for your question.
For context, we are using the header X-MongoDB-Service-Version which should contain the format gitHash=[some git hash]; versionString=[some version e.g. 5.0.0.100.20210710T1827Z]
In case I receive keys I do not support I am ignoring them, for instance gitHash=[some git hash]; versionString=[some version e.g. 5.0.0.100.20210710T1827Z]; keyNotKnown=[value]
I think ignoring keys is an acceptable thing to do, that is exactly what the encoding/json package does if you're missing some fields in your struct.
There was a problem hiding this comment.
Fair enough, if we added logging, the client would get a bunch of warnings constantly as the shape changes as well, which is not a great experience.
Description
Parse service version header
X-MongoDB-Service-VersionLink to any related issue(s): CLOUDP-82836
Type of change:
Required Checklist:
make fmtand formatted my codeFurther comments