Use human-readable timestamp (like ISO-8601)
Closed this issue · 17 comments
What is the issue and why is it an issue?
Right now GBFS uses seconds since the epoch as a timestamp, for example in the last_updated field. While this practice is pretty wide-spread, it makes debugging feeds a little harder than necessary.
Every time I need to check the last_updated field I need to copy the epoch seconds, google "epoch to time converter", paste the seconds to get a usable date time.
While this isn't an impossible task, I think there are alternatives that are both human-readable and unique so that machines can identify a universal point in time.
Please describe some potential solutions you have considered (even if they aren’t related to GBFS).
I suggest that GBFS uses ISO-8601 date times. An example would be 2023-07-06T11:35:33+0200.
This is much easier to interpret for a human and can be parsed easily by many programming languages.
This is of course a major breaking change so it can only be introduced for a major version jump.
Is your potential solution a breaking change?
- Yes, very much so
- No
- Unsure
For what it's worth, we had a lengthy discussion on this early on in #9. I think the consensus was that we should switch to RFC 3339 with a timezone of "Z" (i.e. UTC) but at the time we lacked versioning and since it would have broken existing feeds we decided to table it.
Would appreciate this change, let's introduce it in 3.0.
If I understand it correctly then RFC3339 is a subset of ISO 8601 which reduces the number of allowed formats, isn't it? That would be great since ISO gives quite a bit of flexibility.
With two people in favour I believe this might have a chance of making it into GBFS 3.
Who else do I need to convince?
About only allowing UTC as the timezone: the Java parser, which I would use, can deal with any offset so I don't have a strong opinion either way but it's probably better to be restrictive early on and then allow more flexibility later, if there is a need.
UTC makes sense to me, there is no need for the local timezone because this data is not user focussed.
Hi all! A reminder of the governance process, especially if we want to move quickly to roll this into v3.0 (which would be v3.0-RC2, also including #497). A Pull Request must be open for 7 calendar days before we can open a vote, so if we open a PR today, we can call a vote on Thursday, July 13th.
@leonardehrenfried would you like to be responsible for taking the advocate role? Meaning you open the PR and call the vote, or would you like MobilityData to take that role?
I would! Do you think it makes sense to do it as early as possible or should I gather some feedback here to see if I meet any resistance?
We could open it as early as possible since we still require 7 days of discussion over at the PR.
Tagging some stakeholders to get more discussion going either here or once the PR is opened.
@kulovan @cmonagle @futuretap @ezmckinn @bdferris-v2 @hynick4 @benwedge @testower @fbouchPBSC @rickbruce
@AntoineAugusti @Cj-Malone
Is there any downside from a technical performance aspect? I assume 99.9% of the time the feeds are consumed by machines. I do like the human-readable proposal as it would also make my life slightly easier as I monitor feed quality, much in the way @leonardehrenfried has to run the timestamp through a converter.
I am onboard with it being UTC to start, however I would recommend we include the UTC timezone in the timestamp so that we could expand the options in a minor release.
Downsides:
- I can imagine that in some languages parsing it is harder than epoch seconds and I hope that people won't start writing regexes. In Java it's trivial to parse.
- It's different than before so has some switching costs.
Yes, I agree. I would specify something like "RFC3339 but timezone must always be Z".
I don’t really have any objection to this. There will be a performance hit for parsing for caching purposes, but probably doesn’t outweigh benefits of readability.
As for only allowing utc time, I’m not sure why this restriction is necessary; parser are able to handle offsets, and if the purpose is readability, local times are much more useful.
These are my two cents, anyway 🙂
As for only allowing utc time, I’m not sure why this restriction is necessary; parser are able to handle offsets, and if the purpose is readability, local times are much more useful.
I'm starting to think it's worth going all-in from the start of 3.0 if we're going to do this anyway. Would love to hear from the perspective of a consumer.
If we are going to do timezones it should match what is in the system information file, or we omit it here and rely on the timezone from that file
For debugging feeds it‘s much easier indeed to work with RFC 3339 dates.
I just hope it’s going to be adopted correctly. We’ve already so many exceptions in our parser for non-compliant feeds and I hope we don’t have to add even more!
or we omit it here and rely on the timezone from that file
I think that's too risky. The timestamp should definitely include the offset. The spec could maybe say that it SHOULD match the timezone from system_information.json
I also think that we should insist on a timezone, or offset to be precise.
I personally don't think we need to restrict it to UTC (because you will have to convert it to the representation that you need but it seems that it would reduce a lot of discussions so I went with it.
I have opened #522 which implements this change.
Look forward to the discussion over in the PR.
Closing so that all discussion now takes place at #522