potiuk commented on code in PR #23189:
URL: https://github.com/apache/airflow/pull/23189#discussion_r856938871
##########
dev/breeze/src/airflow_breeze/utils/run_utils.py:
##########
@@ -91,9 +91,9 @@ def run_command(
console.print("[red]========================= STDERR start
============================[/]")
console.print(ex.stderr)
console.print("[red]========================= STDERR end
==============================[/]")
- if not check:
+ if check:
raise
- return None
+ return ex
Review Comment:
Because `CalledProcessorError` has the same fields as `CompletedProcess` :)
Both have:
* returncode
* stdout
* stderr
And then, we can handle it much easier (and it makes handling returned types
much easier when we use `check=False`) - instead of
`Optional[CompletedProcess]` we return now
`Union[CalledProcessError,CompletedProcess`]. Then we can do:
```
if result.returncode != 0:
......
sys.exit(result.returncode)
```
instead of something like that:
```
if not result or returncode != 0:
......
sys.exit(result.returncode if result else 1)
```
This makes all the conditionals far easier.
One could ask why we need to get returncode and `check = True`. The main
reason why we do it because this is a "command line tool".
So rather than printing the user an ugly stacktrace we really want to print
a nice and explanatory error message (nice with [red] marker) so we just need
to exit with the right returncode rather than throw ugly exception. This is in
cases where we "expect" some errors (for example we expect that "building docs"
might fail - but we already handle it by printing all reasons and errors.
We still have a number of cases when we do "check=True" - this is when we do
not expect the command to fail ever (but for many reasons - misconfigured
system etc it might happen). In this case we simply run `check=True` and if
such command fails, ugly exception will be printed (ugly but useful for
diagnostics in this case :).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
