rki-covid-api icon indicating copy to clipboard operation
rki-covid-api copied to clipboard

Wrong lastUpdate time due to time change

Open mmkk19 opened this issue 3 years ago • 12 comments

Seems like time change on Sunday has affected the lastUpdate time, see e.g. Germany https://api.corona-zahlen.org/germany It is now "lastUpdate":"2021-03-28T23:00:00.000Z" though the data is from today (29.03.)

Is this an error by RKI or can anyone fix this?

mmkk19 avatar Mar 29 '21 20:03 mmkk19

lastUpdate is the UTC time. When you actually parse it in your timezone, the correct time is displayed (March 29, 1am). Look at the footer at https://covid.beyerleinf.de :)

beyerleinf avatar Mar 29 '21 20:03 beyerleinf

lastUpdate is the UTC time. When you actually parse it in your timezone, the correct time is displayed (March 29, 1am). Look at the footer at https://covid.beyerleinf.de :)

You are right. RKI is updating the data at "01:00:00,000 German time" every night! (bevor and after switching to DST (daylight saving time) bevor "01:00:00,000 (German Time)" was "(date from today);00:00:00,000Z" (the Z meens "zulu" = 0 =GMT=UTC) because German time was UTC(or GMT)+1 afther switching to DST, "01:00:00,000 (German Time)" is "(date from yesterday);23:00:00,000Z" because German time is now UTC(or GMT)+2 Thats not a error from the API! and not from RKI! i think this issue can be closed.

Rubber1Duck avatar Mar 30 '21 06:03 Rubber1Duck

An advise to every developer to use only UTC time internally!

ChristophKrause avatar Mar 30 '21 08:03 ChristophKrause

While I totally get your points with the UTC, I still think this should not be the intended behavior. This is also highly inconsistent: /districts still shows the "correct" date with "lastUpdate":"2021-03-30T00:00:00.000Z (today) like it used to be before.

Anyway, if the majority says it's ok, we have to deal with it.

mmkk19 avatar Mar 30 '21 17:03 mmkk19

While I totally get your points with the UTC, I still think this should not be the intended behavior. This is also highly inconsistent: /districts still shows the "correct" date with "lastUpdate":"2021-03-30T00:00:00.000Z (today) like it used to be before.

Anyway, if the majority says it's ok, we have to deal with it.

That's still the UTC time, just checked/updated at a different time. You should not treat this like this is the german time for example.

However, why /districts returns a different value I'm not sure and I'll try and debug it

beyerleinf avatar Mar 30 '21 18:03 beyerleinf

So, just fired up a local instance and I've found some things.

  1. The lastUpdate for /germany currently returns "2021-03-29T23:00:00.000Z" (which would be 30.03. 01:00 in German time) where it should actually return "2021-03-29T22:00:00.000Z" (which would be 30.03. 00:00 in German time). This is because the lastUpdate is taken from the states response which has an hour added to it (for whatever reason). This is the code: https://github.com/marlon360/rki-covid-api/blob/cdb96df94661856da6f79ad77d7dfc11f9d867c7/src/data-requests/states.ts#L35 This could be fixed by removing the added hour for /germany and /states.
  2. /districts however has the wrong value ("2021-03-30T00:00:00.000Z") because there is actually no timezone offset in the docker container. Since the RKI can't provide consistent data, the date string from the response ("30.03.2021, 00:00 Uhr") will be interpreted as it is. The epoch timestamps in the other responses are already in UTC format, hence they are correct.

I can create a PR to fix the date parsing so that every response returns the correct UTC date :)

beyerleinf avatar Mar 30 '21 21:03 beyerleinf

So, just fired up a local instance and I've found some things.

1. The `lastUpdate` for /germany currently returns `"2021-03-29T23:00:00.000Z"` (which would be 30.03. 01:00 in German time) where it should actually return `"2021-03-29T22:00:00.000Z"` (which would be 30.03. 00:00 in German time). This is because the `lastUpdate` is taken from the states response which has an hour added to it (for whatever reason). This is the code: https://github.com/marlon360/rki-covid-api/blob/cdb96df94661856da6f79ad77d7dfc11f9d867c7/src/data-requests/states.ts#L35
    This could be fixed by removing the added hour for `/germany` and `/states`.

so we have first to know what the intention was to add that hour! (maybe to bringt it to "date_of_todayT00:00:00.000Z"; bevor the timechange it was so!)

2. `/districts` however has the wrong value (`"2021-03-30T00:00:00.000Z"`) because there is actually no timezone offset in the docker container. Since the RKI can't provide consistent data, the date string from the response (`"30.03.2021, 00:00 Uhr"`) will be interpreted as it is. The epoch timestamps in the other responses are already in UTC format, hence they are correct.

setting a time zone in the docker container will not affect the UTC time for /districts! just tried it. The Date is build by Date() function witch will allways bring the UTC time (as i know)

I can create a PR to fix the date parsing so that every response returns the correct UTC date :)

As written above, we first have to know whats the goal for the dates are! and when patching think about last weekend in Octobre when the time is switching back! :-) I am shure we need this api until next year ...... :-(

Thats all only my 2 cent .......

Rubber1Duck avatar Mar 31 '21 18:03 Rubber1Duck

Anyway, if the majority says it's ok, we have to deal with it.

i dont see any majority here ..........

Rubber1Duck avatar Mar 31 '21 18:03 Rubber1Duck

@Rubber1Duck I agree, we need to understand why @marlon360 added the hour to the /states response.

Yes, setting a Timezone in the Docker container doesn't work, however, I made a small implementation yesterday using DayJS to parse the dates which allows you to tell it which timezone the date should be parsed in. You can check out the changes I made here: https://github.com/marlon360/rki-covid-api/compare/v2...beyerleinf:v2 :)

In my opinion, it would be a good idea to adopt these changes as there is no standardized way the dates by the RKI are handled right now

beyerleinf avatar Mar 31 '21 18:03 beyerleinf

@beyerleinf I had a look on it, and it looks fine! but i'm unsure if this is what all api users want to see ............ lets see whats @marlon360 opinion is ...........

by the way in my view there is another issue at all history data and there last update time history This is for all history data because the lastUpdate value is always the value from the last entry

Rubber1Duck avatar Mar 31 '21 20:03 Rubber1Duck

I think providing a timestamp for the history is fine. Actually changing this now could break the API for a lot of users (I would assume at least).

But I agree that lastUpdate for the history should not be the last entry in the history array. Then again, it's down to interpretation. It would be more concise though if this was the actual date (in your example 2021-03-31, as you said).

beyerleinf avatar Mar 31 '21 20:03 beyerleinf

Any news on this?

beyerleinf avatar Apr 20 '21 20:04 beyerleinf

fixed

Rubber1Duck avatar Oct 18 '22 20:10 Rubber1Duck