adds std pipe iterator for stdin and stderr #1321
Conversation
✅ Deploy Preview for dlt-hub-docs canceled.
|
dlt/common/logger.py
Outdated
| @@ -1,3 +1,4 @@ | |||
| import sys | |||
There was a problem hiding this comment.
Was this import added by accident?
| @@ -24,6 +25,45 @@ def exec_to_stdout(f: AnyFun) -> Iterator[Any]: | |||
| print(encode_obj(rv), flush=True) | |||
|
|
|||
|
|
|||
| def iter_std(venv: Venv, command: str, *script_args: Any) -> Iterator[Tuple[int, str]]: | |||
| """Starts a process `command` with `script_args` in environment `venv` and returns iterator | |||
| of (filno, line) tuples where `fileno` is 1 for stdout and 2 for stderr. `line` is | |||
There was a problem hiding this comment.
Maybe we should create a simple enum to denote 1 for stdout and 2 as sdterr.
There was a problem hiding this comment.
I will create a Literal with those codes, they correspond to unix file numbers for std so IMO they are fine
dlt/common/runners/stdout.py
Outdated
| def _r_q(std_: int) -> None: | ||
| stream_ = process.stderr if std_ == 2 else process.stdout | ||
| for line in iter(stream_.readline, ""): | ||
| q_.put((std_, line[:-1] if line[-1] == "\n" else line)) |
There was a problem hiding this comment.
I think line.strip() is simpler.
There was a problem hiding this comment.
Also can line in theory be None?
There was a problem hiding this comment.
I think None stops the iterator so no. I could not find it in my tests
| t_out.start() | ||
| t_err = Thread(target=_r_q, args=(2,), daemon=True) | ||
| t_err.start() | ||
| while line := q_.get(): |
There was a problem hiding this comment.
Should we have some hard timeout after it that we shut down or do we just let users use Ctrl+C?
There was a problem hiding this comment.
not now I think. adding timeout is trivial if we need it
dlt/common/runners/stdout.py
Outdated
| @@ -24,6 +25,45 @@ def exec_to_stdout(f: AnyFun) -> Iterator[Any]: | |||
| print(encode_obj(rv), flush=True) | |||
|
|
|||
|
|
|||
| def iter_std(venv: Venv, command: str, *script_args: Any) -> Iterator[Tuple[int, str]]: | |||
There was a problem hiding this comment.
I know that shlex is not windows friendly, I am just wondering if we should sanitize script arguments, wdyt?
There was a problem hiding this comment.
I'm not sure what I should do here? @sultaniman
There was a problem hiding this comment.
This was just a security related concern that something might get injected via arguments which will lead RCE or LCE
Description
An iterator that reads both stdout and stderr line by line from a process run via
Venv