hotfix: fix time format issue #4108
Conversation
ts/nni_manager/common/log.ts
Outdated
| // `time.toLocaleString('sv')` trick does not work for Windows | ||
| const isoTime = new Date(new Date().toLocaleString() + ' UTC').toISOString(); | ||
| const time = isoTime.slice(0, 10) + ' ' + isoTime.slice(11, 19); | ||
| const jsonTime = new Date().toJSON(); |
There was a problem hiding this comment.
This will print UTC time. In log we want local time.
The trick above has been tested in en_US and zh_CN locales, but there are hundreds of other locales over the world.
It would be nice if you can find a better method to do the converting.
There was a problem hiding this comment.
My fault. Didn't notice that toJSON() prints UTC time. But the previous trick seems didn't work on en_US windows machines and zh_CN mac machines. I have updated the pr to calculate a correct time format by our own.
| const now = new Date(); | ||
| const date = now.getFullYear() + '-' + zeroPad(now.getMonth() + 1) + '-' + zeroPad(now.getDate()); | ||
| const time = zeroPad(now.getHours()) + ':' + zeroPad(now.getMinutes()) + ':' + zeroPad(now.getSeconds()); | ||
| const datetime = date + ' ' + time; |
There was a problem hiding this comment.
Looks really tedious. But I guess there's no better way.
There was a problem hiding this comment.
Yes. The only better way may be using a third-party strftime library.
|
Have you tried |
Yes, node 16.x could work while 12.x couldn't. Seems that the format of this function changes a lot across different versions. If we use this function, we must make sure to use the node under |
|
Shoule we create a hotfix version wheel (like 2.4.1?) to pip to avoid affecting more users? @liuzhe-lz |
We should not use Date().toLocaleString() to get a formatted time string since it uses the local time format on user's machine and the format is different across platforms (windows, linux, mac) even node versions. This will lead to a failure when starting the rest server. Instead, we construct the time format by ourselves (since using libraries like
strftimerequire extra dependences) which should be consistent across different platforms.BTW, not sure why we add a ' UTC' to a local time and convert it to a UTC time. This will lead to a incorrect time if you are not in +0000.