Skip to content
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

[SPARK-31232][SQL][DOCS] Specify formats of spark.sql.session.timeZone #27999

Closed
wants to merge 2 commits into from

Conversation

MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Mar 24, 2020

What changes were proposed in this pull request?

In the PR, I propose to update the doc for spark.sql.session.timeZone, and restrict format of config's values to 2 forms:

  1. Geographical regions, such as America/Los_Angeles.
  2. Fixed offsets - a fully resolved offset from UTC. For example, -08:00.

Why are the changes needed?

Other formats such as three-letter time zone IDs are ambitious, and depend on the locale. For example, CST could be U.S. Central Standard Time and China Standard Time. Such formats have been already deprecated in JDK, see Three-letter time zone IDs.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running ./dev/scalastyle, and manual testing.

@MaxGekk
Copy link
Member Author

MaxGekk commented Mar 24, 2020

@cloud-fan @HyukjinKwon Please, review this PR.

@SparkQA
Copy link

SparkQA commented Mar 24, 2020

Test build #120258 has finished for PR 27999 at commit 147dcdd.

  • This patch fails PySpark pip packaging tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MaxGekk
Copy link
Member Author

MaxGekk commented Mar 24, 2020

jenkins, retest this, please

@SparkQA
Copy link

SparkQA commented Mar 24, 2020

Test build #120281 has finished for PR 27999 at commit 147dcdd.

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

val SESSION_LOCAL_TIMEZONE = buildConf("spark.sql.session.timeZone")
.doc("The ID of session local timezone in the format of either region-based zone IDs or " +
"zone offsets. Region IDs must have the form 'area/city', such as 'America/Los_Angeles'. " +
"Zone offsets must be in the format '(+|-)HH:mm', for example '-08:00' or '+01:00'.")
Copy link
Member

Choose a reason for hiding this comment

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

Actually why don't we add the simple reason about three-latters timezones? Like you described in PR description: other formats such as three-letter timezone IDs are ambitious

Copy link
Contributor

Choose a reason for hiding this comment

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

good idea!

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed the doc.

@SparkQA
Copy link

SparkQA commented Mar 25, 2020

Test build #120317 has finished for PR 27999 at commit b729f5c.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MaxGekk
Copy link
Member Author

MaxGekk commented Mar 25, 2020

jenkins, retest this, please

@cloud-fan
Copy link
Contributor

This is a doc only change, it should be good to go after pass compilation.

Thanks, merging to master/3.0!

@cloud-fan cloud-fan closed this in 27d53de Mar 25, 2020
cloud-fan pushed a commit that referenced this pull request Mar 25, 2020
In the PR, I propose to update the doc for `spark.sql.session.timeZone`, and restrict format of config's values to 2 forms:
1. Geographical regions, such as `America/Los_Angeles`.
2. Fixed offsets - a fully resolved offset from UTC. For example, `-08:00`.

Other formats such as three-letter time zone IDs are ambitious, and depend on the locale. For example, `CST` could be U.S. `Central Standard Time` and `China Standard Time`. Such formats have been already deprecated in JDK, see [Three-letter time zone IDs](https://docs.oracle.com/javase/8/docs/api/java/util/TimeZone.html).

No

By running `./dev/scalastyle`, and manual testing.

Closes #27999 from MaxGekk/doc-session-time-zone.

Authored-by: Maxim Gekk <max.gekk@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 27d53de)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@SparkQA
Copy link

SparkQA commented Mar 25, 2020

Test build #120322 has finished for PR 27999 at commit b729f5c.

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

sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
### What changes were proposed in this pull request?
In the PR, I propose to update the doc for `spark.sql.session.timeZone`, and restrict format of config's values to 2 forms:
1. Geographical regions, such as `America/Los_Angeles`.
2. Fixed offsets - a fully resolved offset from UTC. For example, `-08:00`.

### Why are the changes needed?
Other formats such as three-letter time zone IDs are ambitious, and depend on the locale. For example, `CST` could be U.S. `Central Standard Time` and `China Standard Time`. Such formats have been already deprecated in JDK, see [Three-letter time zone IDs](https://docs.oracle.com/javase/8/docs/api/java/util/TimeZone.html).

### Does this PR introduce any user-facing change?
No

### How was this patch tested?
By running `./dev/scalastyle`, and manual testing.

Closes apache#27999 from MaxGekk/doc-session-time-zone.

Authored-by: Maxim Gekk <max.gekk@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@MaxGekk MaxGekk deleted the doc-session-time-zone branch June 5, 2020 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants