-
Notifications
You must be signed in to change notification settings - Fork 143
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
[Improvement] Replace ConfigEntry with ConfigOption in RssSparkConfig… #1365
base: master
Are you sure you want to change the base?
Conversation
public static final ConfigEntry<String> RSS_WRITER_PRE_ALLOCATED_BUFFER_SIZE = | ||
createStringBuilder(new ConfigBuilder("spark.rss.writer.pre.allocated.buffer.size")) | ||
.createWithDefault("16m"); | ||
public static final ConfigOption<String> RSS_WRITER_BUFFER_SIZE = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not correct, Please follow this example.
public static final ConfigOption<Long> RSS_CLIENT_SEND_SIZE_LIMITATION =
ConfigOptions.key("rss.client.send.size.limit")
.longType()
.defaultValue(1024 * 1024 * 16L)
.withDescription("The max data size sent to shuffle server");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have already changed it, please help me look at it again.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1365 +/- ##
============================================
+ Coverage 53.35% 54.16% +0.80%
- Complexity 2690 2714 +24
============================================
Files 410 398 -12
Lines 23590 21574 -2016
Branches 2003 2041 +38
============================================
- Hits 12587 11685 -902
+ Misses 10229 9176 -1053
+ Partials 774 713 -61 ☔ View full report in Codecov by Sentry. |
… #1302
What changes were proposed in this pull request?
Following this project's Code of Conduct
Using the ConfigOption to unify all client and server config style.
Why are the changes needed?
Unify all client and server config style.
Does this PR introduce any user-facing change?
How was this patch tested?