[GitHub] [airflow] potiuk commented on a diff in pull request #22901: Catch error in Breeze when docker is not running

[GitBox <gi...@apache.org>]

potiuk commented on code in PR #22901:
URL: https://github.com/apache/airflow/pull/22901#discussion_r847195377


##########
dev/breeze/src/airflow_breeze/utils/docker_command_utils.py:
##########
@@ -179,11 +179,7 @@ def check_docker_version(verbose: bool):
             text=True,
         )
         if not docker_version_output:
-            console.print(
-                '[red]ERROR[/] Docker version could not be determined. Please make sure Docker is installed '
-                'and running.'
-            )
-            return
+            raise Exception("No running Docker instance found")

Review Comment:
   Yeah. This is a CLI tool so at any point when we raise exception, it should only happen when there are "unknown" error conditions. 
   
   What we usually do in other parts of the breeze code is:
   
   1) use rich's console (from utils)
   2) write the error with `[red[/]" 
   3) provide helpful guidance of what to do in `[bright_yellpw][/]" markers
   4) sys.exit(1) (or other non-zero error)
   
   I have not documented this yet, but we might want to add ADR for that and do some custom rich "styles" (info/error/guidance etc.) to make it more obvious. I will add an issue for that.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org