Repository navigation
change all time fields to have type time.Time - #190
Conversation
… check if they're nil
jrperritt
changed the title
|
I think this addresses most (if not all) of the custom time formats. I'll leave it open until Wednesday if anyone wants to review it. |
|
I have a few small questions: It looks like structures with timestamps get declared as It also looks like Should these two points be detailed in the style guide? Even more, it looks like |
Yes
Thank you for pointing it out. That needs to be changed;
The second is a mistake on my part that I'll correct today. I don't think the first is a matter of style, but one of necessity: for the
Here again I don't think this is a matter of style; However, regardless of where it ends up, I do agree it would be worthwhile to document the process of handling |
| type tmp Volume | ||
| var s struct { | ||
| tmp | ||
| CreatedAt gophercloud.JSONRFC3339MilliNoZ `json:"created_at"` |
There was a problem hiding this comment.
This is probably a silly question but why are we using gophercloud.JSONRFC3339MilliNoZ for CreatedAt and UpdatedAt? As far as I can tell these fields are ISO8601 timestamps and so can contain a "Z". If we set the type to be JSONRFC3339MilliNoZ, json.Unmarshall will error out when processing a date such as "2017-01-04T16:09:33.860887324Z", which should, by my reading, of the specs should be valid.
There's no special marshalling for the CreatedAt and UpdatedAt fields in the images.Image structure yet the openstack API documentation for these fields seems to be identical to documentation for their counterparts in volumes.Volume. Could we perhaps just delete this function and update the tags instead?
There was a problem hiding this comment.
Ah I see. If we use a time.Time here, we will be unable to process dates that do not contain timezone information, which are also valid ISO8601 dates. However, the current code won't handle those dates properly either. It won't return an error when Unmarshalling but the dates will almost always be interpreted incorrectly, unless both client and server are in the UK or Ireland and it's deep midwinter. ISO8601 dates without timezone information are in local time but time.Parse interprets such date strings as being in UTC. I think the following code would do the correct thing in all cases:
t, err := time.Parse(time.RFC3339, s)
if err != nil {
t, err = time.ParseInLocation("2006-01-02T15:04:05", s, time.Local)
}
Perhaps the best thing to do would be to create a new goperhcloud time type that uses the above code in its UnmarshalText method.
|
I was just wondering when this PR will be merged? We're using gophercloud in https://github.com/01org/ciao but cannot update our vendored version of this package due to issues with the way times are currently handled, e.g., Image.UpdateAt is currently mandatory and "2017-01-04T16:09:33.860887324Z" is not currently accepted as a valid value for Volume.CreatedAt. Looking at the code this PR will fix most of our issues (all if we drop the custom Unmarshalling for volumes.Volume). Is there a plan to merge it any time soon? |
…asource Compute v2: Flavor Data Source
4 participants
for #75