Skip to content

Conversation

@HeartSaVioR
Copy link
Contributor

What changes were proposed in this pull request?

This patch addresses the post-hoc review comment linked here - #25670 (comment)

Why are the changes needed?

We would like to explicitly document the direct relationship before we finish up structuring of configurations.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

N/A

@HeartSaVioR
Copy link
Contributor Author

cc. @HyukjinKwon

@SparkQA
Copy link

SparkQA commented Feb 14, 2020

Test build #118399 has finished for PR 27576 at commit 5760af9.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HeartSaVioR
Copy link
Contributor Author

retest this, please

@SparkQA
Copy link

SparkQA commented Feb 14, 2020

Test build #118405 has finished for PR 27576 at commit 5760af9.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

retest this please

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

Thanks @HeartSaVioR.

private[spark] val EVENT_LOG_ROLLING_MAX_FILE_SIZE =
ConfigBuilder("spark.eventLog.rolling.maxFileSize")
.doc("The max size of event log file to be rolled over.")
.doc(s"Precondition: ${EVENT_LOG_ENABLE_ROLLING.key}. " +
Copy link
Member

Choose a reason for hiding this comment

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

Okay, I saw the new PR #27575. We could match how we describe there. Shall we also fix docs/configuration.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! I missed we have equivalent description in doc as well. Will update as well.

@SparkQA
Copy link

SparkQA commented Feb 14, 2020

Test build #118415 has finished for PR 27576 at commit 5760af9.

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

@HeartSaVioR HeartSaVioR force-pushed the SPARK-28869-FOLLOWUP-doc branch from ff2170f to 705d129 Compare February 15, 2020 11:28
@HeartSaVioR
Copy link
Contributor Author

Picked existing style from #27575, and updated here as well.

@SparkQA
Copy link

SparkQA commented Feb 15, 2020

Test build #118474 has finished for PR 27576 at commit 705d129.

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

@HyukjinKwon
Copy link
Member

Thanks @HeartSaVioR for folliwing up this one.

@HyukjinKwon
Copy link
Member

Merged to master and branch-3.0.

HyukjinKwon pushed a commit that referenced this pull request Feb 17, 2020
… for rolling event log

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

This patch addresses the post-hoc review comment linked here - #25670 (comment)

### Why are the changes needed?

We would like to explicitly document the direct relationship before we finish up structuring of configurations.

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

No.

### How was this patch tested?

N/A

Closes #27576 from HeartSaVioR/SPARK-28869-FOLLOWUP-doc.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
(cherry picked from commit 446b2d2)
Signed-off-by: HyukjinKwon <[email protected]>
@HeartSaVioR
Copy link
Contributor Author

Thanks for reviewing and merging!

@HeartSaVioR HeartSaVioR deleted the SPARK-28869-FOLLOWUP-doc branch February 17, 2020 12:19
sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
… for rolling event log

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

This patch addresses the post-hoc review comment linked here - apache#25670 (comment)

### Why are the changes needed?

We would like to explicitly document the direct relationship before we finish up structuring of configurations.

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

No.

### How was this patch tested?

N/A

Closes apache#27576 from HeartSaVioR/SPARK-28869-FOLLOWUP-doc.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
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