New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Be more clear what dynamic ADA does #2586
Conversation
Code coverage for golang is
|
@@ -21,6 +21,8 @@ import ( | |||
"time" | |||
) | |||
|
|||
const timeFormat = "2006-01-02 15:04:05 MST" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why should we use MST
instead of UTC
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. So if we don't have any special reason, we should use UTC to be unified with other places such as the timestamp of the stage log message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't even know that other timestamps in the stage log are in UTC. Let me do that.
Code coverage for golang is
|
Code coverage for golang is
|
PTAL when you guys have time 😄 |
@@ -39,7 +41,8 @@ type DataPoint struct { | |||
} | |||
|
|||
func (d *DataPoint) String() string { | |||
return fmt.Sprintf("timestamp: %q, value: %g", time.Unix(d.Timestamp, 0), d.Value) | |||
// Timestamp is shown in UTC. | |||
return fmt.Sprintf("timestamp: %q, value: %g", time.Unix(d.Timestamp, 0).In(time.UTC).Format(timeFormat), d.Value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return fmt.Sprintf("timestamp: %q, value: %g", time.Unix(d.Timestamp, 0).In(time.UTC).Format(timeFormat), d.Value) | |
return fmt.Sprintf("timestamp: %q, value: %g", time.Unix(d.Timestamp, 0).UTC(), d.Value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Actuallly without the format, it gets a bit difficult to read.
package main
import (
"fmt"
"time"
)
func main() {
timeFormat := "2006-01-02 15:04:05 MST"
var timestamp int64 = 1600000000
fmt.Println(time.Unix(timestamp, 0).In(time.UTC).Format(timeFormat))
// 2020-09-13 12:26:40 UTC
fmt.Println(time.Unix(timestamp, 0).UTC())
// 2020-09-13 12:26:40 +0000 UTC
fmt.Println(time.Unix(timestamp, 0).UTC().Format(timeFormat))
// 2020-09-13 12:26:40 UTC
}
But it's better to use .UTC(), instead of In(time.UTC), so let me apply it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -50,6 +53,11 @@ type QueryRange struct { | |||
To time.Time | |||
} | |||
|
|||
func (q *QueryRange) String() string { | |||
// Timestamps are shown in UTC. | |||
return fmt.Sprintf("from: %q, to: %q", q.From.In(time.UTC).Format(timeFormat), q.To.In(time.UTC).Format(timeFormat)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return fmt.Sprintf("from: %q, to: %q", q.From.In(time.UTC).Format(timeFormat), q.To.In(time.UTC).Format(timeFormat)) | |
return fmt.Sprintf("from: %q, to: %q", q.From.UTC(), q.To.UTC()) |
Fixed! PTAL |
/lgtm |
Code coverage for golang is
|
@khanhtc1202 PTAL :-) |
🚀 |
What this PR does / why we need it:
This PR improves the ADA stage log so that we can see how dynamic ADA works.
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: