Skip to content

Conversation

@tdas
Copy link
Contributor

@tdas tdas commented Dec 6, 2016

What changes were proposed in this pull request?

Easier to read while debugging as a formatted string (in ISO8601 format) than in millis

How was this patch tested?

Updated unit tests

@SparkQA
Copy link

SparkQA commented Dec 6, 2016

Test build #69710 has finished for PR 16166 at commit 095184d.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@zsxwing zsxwing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor issue about the time zone. Otherwise, LGTM.

// The timestamp we report an event that has no input data
private var lastNoDataProgressEventTime = Long.MinValue

private val timestampFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'") // ISO8601
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to set the timezone before using this format. Otherwise, the time is based on your timezone. E.g.,

scala> new java.util.Date()
res7: java.util.Date = Tue Dec 06 10:48:00 PST 2016

scala> val timestampFormat = new java.text.SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'") 
timestampFormat: java.text.SimpleDateFormat = java.text.SimpleDateFormat@5af7aed5

scala> timestampFormat.format(new java.util.Date())
res8: String = 2016-12-06T10:48:09.342Z

scala> val timestampFormat = new java.text.SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'") 
timestampFormat: java.text.SimpleDateFormat = java.text.SimpleDateFormat@5af7aed5

scala> timestampFormat.setTimeZone(java.util.TimeZone.getTimeZone("UTC"))

scala> timestampFormat.format(new java.util.Date())
res10: String = 2016-12-06T18:48:20.048Z

@SparkQA
Copy link

SparkQA commented Dec 7, 2016

Test build #69749 has finished for PR 16166 at commit f2cfc4f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@zsxwing
Copy link
Member

zsxwing commented Dec 7, 2016

LGTM. Merging to master and 2.1.

asfgit pushed a commit that referenced this pull request Dec 7, 2016
…rmatted string instead of millis

## What changes were proposed in this pull request?

Easier to read while debugging as a formatted string (in ISO8601 format) than in millis

## How was this patch tested?
Updated unit tests

Author: Tathagata Das <[email protected]>

Closes #16166 from tdas/SPARK-18734.

(cherry picked from commit 539bb3c)
Signed-off-by: Shixiong Zhu <[email protected]>
@asfgit asfgit closed this in 539bb3c Dec 7, 2016
robert3005 pushed a commit to palantir/spark that referenced this pull request Dec 15, 2016
…rmatted string instead of millis

## What changes were proposed in this pull request?

Easier to read while debugging as a formatted string (in ISO8601 format) than in millis

## How was this patch tested?
Updated unit tests

Author: Tathagata Das <[email protected]>

Closes apache#16166 from tdas/SPARK-18734.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…rmatted string instead of millis

## What changes were proposed in this pull request?

Easier to read while debugging as a formatted string (in ISO8601 format) than in millis

## How was this patch tested?
Updated unit tests

Author: Tathagata Das <[email protected]>

Closes apache#16166 from tdas/SPARK-18734.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants