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]

Reply via email to