Make CommandResult a dataclass

[amaslenn]

Following discussion for #717, make CommandResult a dataclass.

[amaslenn]

@philpep I don't really like how BaseBackend.result() looks now, what do you think about changing its signature from result(self, *args: Any, **kwargs: Any) to something like this:

    def result(
        self,
        exit_code: int,
        command: bytes,
        stdout_bytes: bytes,
        stderr_bytes: bytes,
        stdout: Optional[str] = None,
        stderr: Optional[str] = None,
    ) -> CommandResult:

The code that uses it should be updated of course.

Let me know what do you think.

[philpep]

I think we should keep lazy encoding or decoding, i.e. using properties, because sometime we may want to get binary data which cannot be converted to text.

I think the dataclass should have _stdout and _stdout_bytes and check at __init__ that at least one of them are set. Or make subclasses BinaryCommandResult and TextCommandResult.

[amaslenn]

Why is lazy encoding and decoding are needed? My way of thoughts here was to remove dependency on Backend, which looks redundant to me. So why is it needed here?

With dataclass implementation we always know that parameters are set (at least as None, but that is the task for mypy to detect), so this verification is kinda included.

Current solution doesn't break backward compatibility, at least to the extend it is tested now.

Another solution here would be to do similar to what subprocess.CompletedProcess does: it is generic and can be either bytes of str. We can borrow the approach, but that will change the API.

[philpep]

Lazy encoding/decoding is needed because commands like run("cat /usr/bin/cat") will raise an error if trying to decode. Also IIRC either salt and/or ansible might have weird encoding behavior while returning command output in a json field. Lazy encoding/decoding avoid some error, as long as user doesn't explicitly ask for encoding/decoding.

Yes something like this could be fine:

@dataclass
class CommandResult:
    _stdout: AnyStr
    _stderr: AnyStr

    @property
    def stdout(self) -> str:
        if isinstance(self._stdout, bytes):
            return self._stdout.decode(...)
        return self._stdout

    [...]

[amaslenn]

(took longer than I expected, I was busy)

Applied changes to keep lazy encoding/decoding.
@philpep do you think it is time to change result(self, *args: Any, **kwargs: Any) signature in this PR?

[philpep]

I modified result() to make it simple and changed calls instead. Merged, thanks!