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

[#846] feat(cli): Service's start & stop & restart through Cli. #925

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

slfan1989
Copy link
Collaborator

@slfan1989 slfan1989 commented Jun 4, 2023

What changes were proposed in this pull request?

We support starting and stopping services using CLI commands.

  • Command 1: uniffle --daemon start coordinator
coordinator start success, is running as process 61743.
  • Command 2: uniffle --daemon stop coordinator
coordinator stop success.
  • Command 3: uniffle --daemon status coordinator

If the service is started

coordinator is running as process 62087.

If the service is stoped

coordinator is stopped.

Why are the changes needed?

Supporting service start and stop functionality using CLI commands is part of this PR.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Testing environment verification.

@slfan1989 slfan1989 marked this pull request as draft June 4, 2023 09:05
@slfan1989
Copy link
Collaborator Author

Completed the first version, will continue to improve the code and test it in the testing environment. Currently, it is not ready for review yet.

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2023

Codecov Report

Merging #925 (a14298a) into master (e5e1b30) will increase coverage by 1.14%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #925      +/-   ##
============================================
+ Coverage     53.67%   54.82%   +1.14%     
- Complexity     2521     2526       +5     
============================================
  Files           382      362      -20     
  Lines         21646    19305    -2341     
  Branches       1795     1795              
============================================
- Hits          11619    10584    -1035     
+ Misses         9323     8092    -1231     
+ Partials        704      629      -75     

see 37 files with indirect coverage changes

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

@slfan1989 slfan1989 marked this pull request as ready for review June 21, 2023 12:59
jerqi
jerqi previously approved these changes Jun 21, 2023
Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @slfan1989

@slfan1989
Copy link
Collaborator Author

LGTM, thanks @slfan1989

@jerqi Thank you for helping to review the code! I will add some comments to make the code more readable.

@jerqi
Copy link
Contributor

jerqi commented Jun 23, 2023

LGTM, thanks @slfan1989

@jerqi Thank you for helping to review the code! I will add some comments to make the code more readable.

ok.

@slfan1989 slfan1989 self-assigned this Jul 7, 2023
smallzhongfeng
smallzhongfeng previously approved these changes Jul 9, 2023
Copy link
Contributor

@smallzhongfeng smallzhongfeng left a comment

Choose a reason for hiding this comment

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

LGTM2!

@jerqi
Copy link
Contributor

jerqi commented Jul 12, 2023

@slfan1989 Is this pr ready for merging?

@slfan1989
Copy link
Collaborator Author

@jerqi @smallzhongfeng Can you help to review this pr again? Thank you very much! I and @yl09099 have verified with the production environment.

@yl09099
Copy link
Contributor

yl09099 commented Jul 19, 2023

@jerqi @smallzhongfeng Can you help to review this pr again? Thank you very much! I and @yl09099 have verified with the production environment.

LGTM.

Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks all. Merged to master.

@jerqi jerqi merged commit c4bb758 into apache:master Jul 19, 2023
30 checks passed
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.

5 participants