masterzen / winrm

Command-line tool and library for Windows remote command execution in Go
Apache License 2.0
425 stars 129 forks source link

Support canceling remote processes via `context.Context` #125

Open kgadams opened 2 years ago

kgadams commented 2 years ago

Hi Brice, I would like to have public access to the err member of the Command struct. This would make it possible to reimplement the Client.Run* methods out of package in a way that supports canceling via context.Context. Would you consider a PR to this effect?

masterzen commented 2 years ago

Sorry for the very late answer, but I would prefer to have a PR that adds support for passing a context.Context.

kgadams commented 2 years ago

Okay, fair enough. How do you want to go about this?

masterzen commented 2 years ago

I think the RunWithContext would be enough for this small library. Internally, it's just a matter of checking if we have a context and calling the WithContext whenever needed.

kgadams commented 2 years ago

Is this what you had in mind? The external interface is unaffected, but for all exposed functions we now have an alternative that takes a context.Context as first argument.

PaulFarver commented 2 years ago

@masterzen Would you be interested in a PR, where we pass the context to the http request as well with http.NewRequestWithContext, so we can control cancellation of the entire process.

kgadams commented 2 years ago

@masterzen Would you be interested in a PR, where we pass the context to the http request as well with http.NewRequestWithContext, so we can control cancellation of the entire process.

Hi Paul, I toyed with that as well, however canceling the program on the remote machine requires a successful HTTP REST call, so that did not work out for me.

PaulFarver commented 2 years ago

Hi Paul, I toyed with that as well, however canceling the program on the remote machine requires a successful HTTP REST call, so that did not work out for me.

Hi Klaus-Georg, that's a good point. We should absolutely try not to leave processes hanging in the remote host.

I still think it's an issue that we can't cancel requests, though. If the remote host doesn't respond to http requests we have to wait for the os to give us an io timeout.

Just spitballing, but maybe if we split up the interface, so users could run each step separately with it's own context, so we're not lumping multiple requests into the same call.

Maybe this should be considered a separate issue?