Skip to content

Conversation

@sadikovi
Copy link
Contributor

@sadikovi sadikovi commented Dec 2, 2021

What changes were proposed in this pull request?

This is a follow-up PR to #34596. There were a few comments and suggestions raised after the PR was merged, so I addressed them in this follow-up:

  • Instead of using failOnError, which was confusing as no error was thrown in the method, we use allowTimeZone which has an opposite meaning of failOnError and far more descriptive.
  • I updated a few test names to resolve ambiguity.
  • I changed the tests to use withTempPath as was suggested in the original PR.

Why are the changes needed?

Code cleanup and clarifications.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing unit and integration tests.

@github-actions github-actions bot added the SQL label Dec 2, 2021
@sadikovi
Copy link
Contributor Author

sadikovi commented Dec 2, 2021

cc @MaxGekk @cloud-fan @gengliangwang for review. Thanks!

@SparkQA
Copy link

SparkQA commented Dec 2, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50315/

@SparkQA
Copy link

SparkQA commented Dec 2, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50315/

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in f7dabd8 Dec 2, 2021
@SparkQA
Copy link

SparkQA commented Dec 2, 2021

Test build #145840 has finished for PR 34777 at commit 65cdaa5.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants