drop stdin for exec sync #750
Conversation
| @@ -65,7 +65,6 @@ interface Conmon { | |||
| timeoutSec @1 :UInt64; | |||
| command @2 :List(Text); | |||
| terminal @3 :Bool; | |||
| stdin @4 :Bool; | |||
There was a problem hiding this comment.
It also has to be removed from the client.go
haircommander
force-pushed
the
drop-stdin-exec-sync
branch
from
3c733ba
to
298a34f
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #750 +/- ##
=======================================
Coverage 31.84% 31.84%
=======================================
Files 13 13
Lines 1118 1118
Branches 421 421
=======================================
Hits 356 356
Misses 479 479
Partials 283 283 |
| @@ -709,7 +709,6 @@ func (c *ConmonClient) ExecSyncContainer(ctx context.Context, cfg *ExecSyncConfi | |||
| return err | |||
| } | |||
| req.SetTerminal(cfg.Terminal) | |||
| req.SetStdin(cfg.Stdin) | |||
There was a problem hiding this comment.
I’d say we drop it from the cfg type as well.
There was a problem hiding this comment.
🙃 good catch
There was a problem hiding this comment.
Ah the tests have to be fixed as well.
haircommander
force-pushed
the
drop-stdin-exec-sync
branch
from
298a34f
to
739c771
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, saschagrunert 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 |
as i don't think there's a way it can be passed in Signed-off-by: Peter Hunt~ <pehunt@redhat.com>
haircommander
force-pushed
the
drop-stdin-exec-sync
branch
from
739c771
to
b72a78b
Compare
|
PTAL @saschagrunert if still around |
|
/lgtm |
as i don't think there's a way it can be passed in
Signed-off-by: Peter Hunt~ pehunt@redhat.com
What type of PR is this?
/kind bug
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?