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

[#1476] feat(spark): Provide dedicated unregister app rpc interface #1510

Merged
merged 6 commits into from
Feb 8, 2024

Conversation

somelovelanguage
Copy link
Contributor

What changes were proposed in this pull request?

Introduce dedicated unregisterApp rpc interface which is only called once when unregister shuffle

Why are the changes needed?

Fix: #1476

Does this PR introduce any user-facing change?

No.

How was this patch tested?

UT

Signed-off-by: Yiqi You <2810592075@qq.com>
@somelovelanguage somelovelanguage marked this pull request as draft February 6, 2024 06:36
Signed-off-by: Yiqi You <2810592075@qq.com>
@somelovelanguage somelovelanguage marked this pull request as ready for review February 6, 2024 07:17
@somelovelanguage somelovelanguage marked this pull request as draft February 6, 2024 07:19
@somelovelanguage somelovelanguage marked this pull request as ready for review February 6, 2024 07:22
Copy link
Member

@zuston zuston left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Left some minor comments.

@zuston zuston changed the title [FEATURE] Provide dedicated unregister app rpc interface [#1476] feat(spark): Provide dedicated unregister app rpc interface Feb 6, 2024
Copy link

github-actions bot commented Feb 6, 2024

Test Results

2 287 files  2 287 suites   4h 27m 51s ⏱️
  819 tests   818 ✅  1 💤 0 ❌
9 086 runs  9 073 ✅ 13 💤 0 ❌

Results for commit 84e0434.

♻️ This comment has been updated with latest results.

Signed-off-by: Yiqi You <2810592075@qq.com>
Signed-off-by: Yiqi You <2810592075@qq.com>
@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2024

Codecov Report

Attention: 47 lines in your changes are missing coverage. Please review.

Comparison is base (e3b4618) 0.00% compared to head (84e0434) 55.19%.
Report is 8 commits behind head on master.

Files Patch % Lines
...ffle/client/impl/grpc/ShuffleServerGrpcClient.java 0.00% 14 Missing ⚠️
...pache/uniffle/server/ShuffleServerGrpcService.java 0.00% 14 Missing ⚠️
.../org/apache/uniffle/server/ShuffleTaskManager.java 0.00% 5 Missing and 1 partial ⚠️
...he/uniffle/client/impl/ShuffleWriteClientImpl.java 75.00% 4 Missing and 1 partial ⚠️
...nt/request/RssUnregisterShuffleByAppIdRequest.java 0.00% 4 Missing ⚠️
.../response/RssUnregisterShuffleByAppIdResponse.java 0.00% 2 Missing ⚠️
.../uniffle/server/event/AppUnregisterPurgeEvent.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #1510       +/-   ##
=============================================
+ Coverage          0   55.19%   +55.19%     
- Complexity        0     2809     +2809     
=============================================
  Files             0      410      +410     
  Lines             0    22049    +22049     
  Branches          0     2082     +2082     
=============================================
+ Hits              0    12170    +12170     
- Misses            0     9121     +9121     
- Partials          0      758      +758     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Yiqi You <2810592075@qq.com>
Signed-off-by: Yiqi You <2810592075@qq.com>
Copy link
Member

@zuston zuston left a comment

Choose a reason for hiding this comment

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

LGTM.

@zuston zuston merged commit 9471cba into apache:master Feb 8, 2024
38 checks passed
@zuston
Copy link
Member

zuston commented Feb 8, 2024

Merged. Thanks for your contribution @somelovelanguage .

Welcome to Uniffle community! 🎉

zuston pushed a commit to zuston/incubator-uniffle that referenced this pull request Mar 19, 2024
…face (apache#1510)

### What changes were proposed in this pull request?
Introduce dedicated unregisterApp rpc interface which is only called once when unregister shuffle

### Why are the changes needed?
Fix: [apache#1476](apache#1476)

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
UT
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.

[FEATURE] Provide dedicated unregister app rpc interface
4 participants