-
-
Notifications
You must be signed in to change notification settings - Fork 636
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
"switched" command runner #16792
"switched" command runner #16792
Conversation
[ci skip-build-wheels]
26a15b3
to
b76e5f2
Compare
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!
[ci skip-build-wheels]
[ci skip-build-wheels]
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.
The nailgun
command runner should be able to use this as well:
pants/src/rust/engine/process_execution/src/nailgun/mod.rs
Lines 79 to 91 in c702901
/// | |
/// A command runner that can run local requests under nailgun. | |
/// | |
/// It should only be invoked with local requests. | |
/// It will read a flag marking an `Process` as nailgunnable. | |
/// If that flag is set, it will connect to a running nailgun server and run the command there. | |
/// Otherwise, it will just delegate to the regular local runner. | |
/// | |
pub struct CommandRunner { | |
inner: super::local::CommandRunner, | |
nailgun_pool: NailgunPool, | |
executor: Executor, | |
} |
[ci skip-build-wheels]
I switched this to using generic types instead of |
Will look into a follow-up. |
Introduce
SwitchedCommandRunner
which uses a predicate to choose between two command runners. Then useSwitchedCommandRunner
to switch between thedocker::CommandRunner
and whatever other local command runner is configured.This allows
docker::CommandRunner
to no longer know about any other command runners, thereby re-introducing better encapsulation todocker::CommandRunner
.Closes #16788