Skip to content

[SPARK-26443][CORE] Use ConfigEntry for hardcoded configs for history category. #23384

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

Closed
ueshin wants to merge 4 commits into from
Closed

[SPARK-26443][CORE] Use ConfigEntry for hardcoded configs for history category. #23384

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
613dad0
Use ConfigEntry for hardcoded configs for history category.
ueshin Dec 26, 2018
a364017
Revert a change.
ueshin Dec 28, 2018
a9208ef
Rename.
ueshin Dec 28, 2018
100902f
Revert "Revert a change."
ueshin Dec 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Revert "Revert a change." 
This reverts commit a364017.
  • Loading branch information
@ueshin
ueshin committed Dec 29, 2018
commit 100902f5239d7663c74d7dc15310268e6b45fe2c
2 changes: 1 addition & 1 deletion core/src/main/scala/org/apache/spark/deploy/history/HistoryServerArguments.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private[history] class HistoryServerArguments(conf: SparkConf, args: Array[Strin
|
| spark.history.fs.logDirectory Directory where app logs are stored
| (default: file:/tmp/spark-events)
| spark.history.fs.updateInterval How often to reload log data from storage
Copy link
Contributor

@mgaido91 mgaido91 Dec 28, 2018

Choose a reason for hiding this comment

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

do we need to change this? Seems this pattern is quite used too...

Copy link
Member Author

@ueshin ueshin Dec 28, 2018

Choose a reason for hiding this comment

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

Seems like spark.history.fs.updateInterval is deprecated, but let me revert it for now.

Copy link
Member

@dongjoon-hyun dongjoon-hyun Dec 28, 2018

Choose a reason for hiding this comment

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

Ur, it is good to remove the deprecated one as many as possible in Spark 3.0. When I searched this string during reviews, there were only two instances; here and deprecation checking test case. @mgaido91 , what do you mean by quite used? We already accepted this deprecated one and new one.

Copy link
Contributor

@mgaido91 mgaido91 Dec 28, 2018

Choose a reason for hiding this comment

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

I see only now that it was deprecated, sorry. Then, this is ok, sorry for the comment.

@dongjoon-hyun I meant that camel case notation for the last element is quite widespread also for other configs, so I didn't see the need for changing the notation.

Copy link
Member Author

@ueshin ueshin Dec 29, 2018

Choose a reason for hiding this comment

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

Then should I update to include this change back?

Copy link
Member

@HyukjinKwon HyukjinKwon Dec 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think it's okie. It's deprecated and we should get rid of it in the doc basically. I see no issue. Yea, I think it can be included back. LGTM without that change as well.

| spark.history.fs.update.interval How often to reload log data from storage
| (in seconds, default: 10)
|""".stripMargin)
// scalastyle:on println
Expand Down