-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
pkg/executor: refine the Executor interface #49494
Conversation
Signed-off-by: Yang Keao <[email protected]>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #49494 +/- ##
================================================
+ Coverage 70.9993% 71.6760% +0.6767%
================================================
Files 1368 1420 +52
Lines 399557 419356 +19799
================================================
+ Hits 283683 300578 +16895
- Misses 96070 100043 +3973
+ Partials 19804 18735 -1069
Flags with carried forward coverage won't be shown. Click here to find out more.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lcwangchao, xhebox The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
/retest |
What problem does this PR solve?
Issue Number: close #49490
Problem Summary:
Currently, the
BaseExecutor
and especiallyBaseExecutor.Ctx()
is used everywhere. We need to progressively replaceBaseExecutor
with someNewBaseExecutor
which doesn't have a.Ctx
inside. To reach this goal, I'll use several functions to replaceBase() *BaseExecutor
.What changed and how does it work?
This is a simple PR. It doesn't harm the logic. It adds the following functions to the
Executor
interface:Each executor can still use the
sctx
inside itself by calling.BaseExecutor.Ctx()
, because they have the accurate type of themselves. It enables us to migrate some executors which don't actually useBaseExecutor
.Check List
Tests
Side effects
Documentation
Release note