-
Notifications
You must be signed in to change notification settings - Fork 721
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
PCP: Support to pause a scheduler #1831 #1942
Conversation
Thanks for your contribution. If your PR get merged, you will be rewarded 0 points. |
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.
Thanks for your contribution. If your PR get merged, you will be rewarded 0 points.
why 0 points? It should be 100 points. Our rebot still silly.
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.
Thanks a lot for your contribution, nice work!
It seems a little complex to use two channels. Maybe there are some simple solutions. For example: Add a field to a scheduler's configuration like DelayUntil
which is the end timestamp that the scheduling should be blocked. Return false when calling AllowSchedule
which checks the value of DelayUntil
. Reset the value of DelayUntil
to 0 when resuming a scheduler.
Feel free to have a discussion if you have better ideas.
Thanks for your reply! it seems that my idea is really complex, i will try your suggestion. |
@lhy1024 @shafreeck please review the new commit. |
/run-all-test |
@hrbeuyz24 Thanks for your contribution! And would you mind adding some unit tests? |
@rleungx @shafreeck @lhy1024 |
/run-al-test |
Codecov Report
@@ Coverage Diff @@
## master #1942 +/- ##
==========================================
- Coverage 77.84% 77.75% -0.09%
==========================================
Files 171 171
Lines 17126 17210 +84
==========================================
+ Hits 13332 13382 +50
- Misses 2753 2779 +26
- Partials 1041 1049 +8
Continue to review full report at Codecov.
|
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.
LGTM. Thanks a lot for your contribution and This is pleasant cooperation.
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.
reset LGTM.
/run-all-tests |
Team Trash Brother complete task #1831 and get 100 score, currerent score 100 |
Which problem does this PR solve?
PCP #1831
What have you changed?
DelayUntil
which is the end timestamp that the scheduling should be blocked, add a funcisPaused
which return sheduler's state, when scheduler start a new work loop, it will judge scheduler whether is paused. Set DelayUntil 0 when resume a scheduler.schedule pause <name><delay>
schedule resume <name>
What are the type of the changes?
How has this PR been tested?
manual test and unit test