potiuk commented on code in PR #27008:
URL: https://github.com/apache/airflow/pull/27008#discussion_r1002604817
##########
dev/breeze/src/airflow_breeze/commands/developer_commands.py:
##########
@@ -589,6 +589,9 @@ def run_shell(verbose: bool, dry_run: bool, shell_params:
ShellParams) -> RunCom
)
if command_result.returncode == 0:
return command_result
+ elif command_result.returncode == 1:
+ get_console().print(f"[red]Error {command_result.returncode}
returned[/]")
+ sys.exit(1)
Review Comment:
I think this was a bit misplaced (even if it fixed the problem of course :)
).
I addressed it in #27191 - the return was fine, it was simply not used in
the @click.command decorated CLI method (we really want to sys.exit(1) in the
CLI methods, not in the underlying functions that are called (it allows to make
tests and reuse the underlying functions in various commands as needed.
Speaking of which - I actually merged "enter_shell" and "run_shell" commands
in @27191 as the `run_shell` has been only used in `enter_shell` and we could
easilhy combine them. But then "enter_shell" command was used in a few places
("shell" command and "start-airflow" command), so it makes more sense to return
the errorcode from "enter_shell" and only sys.exit() in the "decorated" CLI
methods that uses the "enter_shell".
--
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]