Skip to content

[SPARK-26389][SS][FOLLOW-UP]Format config name to follow the other boolean conf naming convention #26981

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[SPARK-26389][SS][FOLLOW-UP]Format config name to follow the other boolean conf naming convention #26981

wants to merge 1 commit into from

Conversation

Ngone51
Copy link
Member

@Ngone51 Ngone51 commented Dec 23, 2019 •
edited
Loading

What changes were proposed in this pull request?

Rename spark.sql.streaming.forceDeleteTempCheckpointLocation to spark.sql.streaming.forceDeleteTempCheckpointLocation.enabled.

Why are the changes needed?

To follow the other boolean conf naming convention.

Does this PR introduce any user-facing change?

No, as this config is newly added in 3.0.

How was this patch tested?

Pass Jenkins.

@Ngone51 Ngone51 mentioned this pull request Dec 23, 2019
Closed
@Ngone51
Copy link
Member Author

Ngone51 commented Dec 23, 2019

cc @gatorsmile @cloud-fan @gaborgsomogyi @HeartSaVioR

@HeartSaVioR
Copy link
Contributor

HeartSaVioR commented Dec 23, 2019 •
edited
Loading

It would be great if we make clear about "Does this PR introduce any user-facing change?"; which versions are affected by such incompatible change. If official versions are contained we should also link both via AlternateConfig.

I know that's only applied to 3.0.0 so I don't expect we want to use AlternateConfig to couple them, but given the description of PR describes "yes" on the statement, so better to be clear about the reason of "yes". (Given this is FOLLOW-UP of the feature which is not released officially, maybe "No" is OK as well.)

HeartSaVioR
HeartSaVioR approved these changes Dec 23, 2019
View reviewed changes
Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

Other than PR description, the code change LGTM.

@Ngone51
Copy link
Member Author

Ngone51 commented Dec 23, 2019 •
edited
Loading

Given this is FOLLOW-UP of the feature which is not released officially, maybe "No" is OK as well.

Make sense. And thank you for your quick response.

@cloud-fan
Copy link
Contributor

ok to test

@cloud-fan
Copy link
Contributor

@Ngone51 can you update the PR description and explicitly mention that this config is newly added in 3.0?

@Ngone51
Copy link
Member Author

Ngone51 commented Dec 23, 2019

@Ngone51 can you update the PR description and explicitly mention that this config is newly added in 3.0?

updated, thanks!

@cloud-fan
Copy link
Contributor

retest this please

1 similar comment
@cloud-fan
Copy link
Contributor

retest this please

@Ngone51
Copy link
Member Author

Ngone51 commented Dec 23, 2019

It seems that Jenkins has crashed for the whole day...

@SparkQA
Copy link

SparkQA commented Dec 23, 2019

Test build #115646 has finished for PR 26981 at commit b261827.

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

@cloud-fan
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Dec 24, 2019

Test build #115693 has finished for PR 26981 at commit b261827.

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

@HeartSaVioR
Copy link
Contributor

retest this, please

@HeartSaVioR
Copy link
Contributor

FYI #27001 (SPARK-30345) is raised to fix the test build 115646 (https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/115646/testReport)

@SparkQA
Copy link

SparkQA commented Dec 24, 2019

Test build #115709 has finished for PR 26981 at commit b261827.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 6d64fc2 Dec 25, 2019
@Ngone51
Copy link
Member Author

Ngone51 commented Dec 26, 2019

Thanks all!

fqaiser94 pushed a commit to fqaiser94/spark that referenced this pull request Mar 30, 2020
@Ngone51 @fqaiser94
[SPARK-26389][SS][FOLLOW-UP] Format config name to follow the other b…
bc3f09a 
…oolean conf naming convention

### What changes were proposed in this pull request?

Rename `spark.sql.streaming.forceDeleteTempCheckpointLocation` to `spark.sql.streaming.forceDeleteTempCheckpointLocation.enabled`.

### Why are the changes needed?

To follow the other boolean conf naming convention.

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

No, as this config is newly added in 3.0.

### How was this patch tested?

Pass Jenkins.

Closes apache#26981 from Ngone51/SPARK-26389-FOLLOWUP.

Authored-by: yi.wu <yi.wu@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HeartSaVioR HeartSaVioR HeartSaVioR approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ngone51 @HeartSaVioR @cloud-fan @SparkQA