Fix session endpoint - #584
Conversation
5069bb8 to
38839ba
Compare
38839ba to
88b707b
Compare
| ServiceType: se.ServiceType, | ||
| ProviderCountry: se.ProviderCountry, | ||
| DateStarted: se.Started.Format("2018-10-29 16:22:05"), | ||
| DateStarted: se.Started.Format("2006-01-02 15:04:05"), |
There was a problem hiding this comment.
It looked okey from the first sight. And then I started thinking (maybe this is the real problem 😄 )
What timezone this format uses? What timezone we use when we set actual session start time?
Maybe its better to use one of predefined ISO formats with UTC time zone? In that case session times will be comparable in whole world.
These are not change requests but more like answers we need to answer and to do that maybe we have to look deeper. This looks like good opportunity to do that
There was a problem hiding this comment.
I've updated the format to RFC3339 which is basicly a stricter ISO 8601.
fde035a to
459ca08
Compare
| ServiceType: se.ServiceType, | ||
| ProviderCountry: se.ProviderCountry, | ||
| DateStarted: se.Started.Format("2018-10-29 16:22:05"), | ||
| DateStarted: se.Started.UTC().Format(time.RFC3339), |
There was a problem hiding this comment.
I think it would be better to set time in UTC before persisting session itself, not when rendering in presentation layer. I assume RFC3339 formatting includes timezone too?
There was a problem hiding this comment.
It does - any reason why you don't want the UTC time there?
There was a problem hiding this comment.
Start time in UTC must be set on session creation - my point is, let's keep times in UTC in database too.
459ca08 to
e8e17c2
Compare
| ServiceType: se.ServiceType, | ||
| ProviderCountry: se.ProviderCountry, | ||
| DateStarted: se.Started.Format("2018-10-29 16:22:05"), | ||
| DateStarted: se.Started.Format(time.RFC3339), |
There was a problem hiding this comment.
We could also use with timezone specifying explicit TZ as: 2006-01-02 15:04:05-0700
Question is, will other peer know how to interpret this, how this agreement on format is passed?
I ALWAYS use unix timestamp in UTC zone. Peer will parse representation for itself as it wishes.
Bonus for this, you can stuff unix timestamp into long type and operate with time arithmetics more easily.
There was a problem hiding this comment.
You'll be able to parse the RFC3339 in any language easily. It does provide timezone information. One thing that unix timestamp is lacking is the human readability. RFC3339 is both human comprehendible and machine readable.
There was a problem hiding this comment.
I would say its a good time to introduce UTC unix timestamp, since we are changing format anyways. Will reduce complexity for users later on.
There was a problem hiding this comment.
@zolia golden rule with time - never ever do direct arithmetics with times. https://en.wikipedia.org/wiki/Leap_second
There was a problem hiding this comment.
I think the RFC3339 is still the way to go. You said it youself - peer will have to parse the timestamp to make sense of it. Why have that step as extra? If you're parsing anyway, you'll be able to parse RFC3339 with the same ease as a timestamp.
IETF seems to recommend RFC3339 as well:
https://tools.ietf.org/html/rfc7493#section-4.3
There was a problem hiding this comment.
Ok, theres cons and pros for that. RFC3339 is representative format as common JSON is too. I see a bunch of pitfalls in such usage, but since event IETF recommends that.. well :)
4 participants
Closes #583
Found this while refactoring the session storage.