Skip to content
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

[ISSUE-300] Make config type of RSS_CLIENT_TYPE as enum #310

Merged
merged 7 commits into from
Nov 28, 2022

Conversation

selectbook
Copy link
Contributor

@selectbook selectbook commented Nov 11, 2022

What changes were proposed in this pull request?

move the ClentType.java from internal-client module to common module
change stringType to enumType for RSS_CLIENT_TYPE in the RssBaseConf.java
use name method convert ShuffleServerConf.RSS_CLIENT_TYPE to String(I am not sure if this is the right way)

Why are the changes needed?

Easy to add different extensions #300

Does this PR introduce any user-facing change?

No

How was this patch tested?

Already added

@jerqi jerqi requested a review from zuston November 11, 2022 18:08
@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2022

Codecov Report

Merging #310 (1dd681d) into master (57a834b) will increase coverage by 0.00%.
The diff coverage is 83.33%.

@@            Coverage Diff            @@
##             master     #310   +/-   ##
=========================================
  Coverage     58.46%   58.47%           
- Complexity     1572     1573    +1     
=========================================
  Files           193      194    +1     
  Lines         10846    10848    +2     
  Branches        954      954           
=========================================
+ Hits           6341     6343    +2     
  Misses         4131     4131           
  Partials        374      374           
Impacted Files Coverage Δ
...org/apache/spark/shuffle/RssSparkShuffleUtils.java 57.14% <0.00%> (ø)
...ain/java/org/apache/uniffle/common/ClientType.java 100.00% <ø> (ø)
...uniffle/storage/factory/ShuffleHandlerFactory.java 0.00% <ø> (ø)
...he/uniffle/client/impl/ShuffleWriteClientImpl.java 29.82% <100.00%> (ø)
.../org/apache/uniffle/common/config/RssBaseConf.java 93.42% <100.00%> (ø)
...a/org/apache/uniffle/server/RegisterHeartBeat.java 87.50% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jerqi
Copy link
Contributor

jerqi commented Nov 18, 2022

@selectbook Will you continue this pr?

@selectbook
Copy link
Contributor Author

@selectbook Will you continue this pr?

yeah, i will continue to fix it from this night

@jerqi
Copy link
Contributor

jerqi commented Nov 19, 2022

@selectbook Will you continue this pr?

yeah, i will continue to fix it from this night

@selectbook Ok, take your time.

@jerqi
Copy link
Contributor

jerqi commented Nov 23, 2022

@selectbook Gently ping.

@jerqi jerqi linked an issue Nov 26, 2022 that may be closed by this pull request
3 tasks
@jerqi jerqi requested a review from zuston November 26, 2022 16:22
@jerqi
Copy link
Contributor

jerqi commented Nov 26, 2022

@selectbook Maybe you're busy now, I help you solve the code style and review issues. I hope you don't mind it. This pr is related to #133. I hope this pr can be merged quickly.

@zuston zuston merged commit ed2c00e into apache:master Nov 28, 2022
@zuston
Copy link
Member

zuston commented Nov 28, 2022

Merged. Thanks @selectbook @jerqi .

@selectbook
Copy link
Contributor Author

@selectbook Maybe you're busy now, I help you solve the code style and review issues. I hope you don't mind it. This pr is related to #133. I hope this pr can be merged quickly.

I'm sorry I've been really busy recently. Thanks for your patience

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.

[Improvement] Make config type of RSS_CLIENT_TYPE as enum
4 participants