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


##########
BREEZE.rst:
##########
@@ -549,12 +548,11 @@ command but it is very similar to current ``breeze`` 
command):
 
 Another way is to exec into Breeze terminal from the host's terminal. Often 
you can
 have multiple terminals in the host (Linux/MacOS/WSL2 on Windows) and you can 
simply use those terminals
-to enter the running container. It's as easy as launching ``./breeze-legacy 
exec`` while you already started the
+to enter the running container. It's as easy as launching ``./breeze exec`` 
while you already started the

Review Comment:
   ```suggestion
   to enter the running container. It's as easy as launching ``breeze exec`` 
while you already started the
   ```



##########
BREEZE.rst:
##########
@@ -566,6 +564,12 @@ command and it is not yet available in the current 
``breeze`` command):
     </div>
 
 
+Those are all available flags of ``build-prod-image`` command:

Review Comment:
   ```suggestion
   Those are all available flags of ``exec`` command:
   ```



##########
dev/breeze/src/airflow_breeze/breeze.py:
##########
@@ -1486,5 +1490,35 @@ def __init__(self, **kwargs):
         width=os.environ.get('RECORD_BREEZE_WIDTH'),
     )
 
+
[email protected](
+    name='exec',
+    help='Joins the interactive shell of running airflow container'
+    )
+@option_verbose
+@option_dry_run
[email protected]('exec_args', nargs=-1, type=click.UNPROCESSED)
+def exec(verbose: bool, dry_run: bool, exec_args: Tuple):
+    container_running = find_airflow_container(verbose=verbose, 
dry_run=dry_run)
+    cmd_to_run = [
+        "docker",
+        "exec",
+        "-it",
+        container_running,
+        "/opt/airflow/scripts/docker/entrypoint_exec.sh",
+    ]
+
+    if exec_args:
+        cmd_to_run.extend(exec_args)
+    run_command(
+        cmd_to_run,
+        verbose=verbose,
+        dry_run=dry_run,
+        check=False,
+        no_output_dump_on_exception=True,

Review Comment:
   ```suggestion
           no_output_dump_on_exception=False,
   ```
   
   I tihnk this is useful if exec fails. I also added a little better handling 
in case of some run_commands - with "nicer" message but I will handle that when 
I merge my PR.



##########
dev/breeze/src/airflow_breeze/breeze.py:
##########
@@ -1486,5 +1490,35 @@ def __init__(self, **kwargs):
         width=os.environ.get('RECORD_BREEZE_WIDTH'),
     )
 
+
[email protected](
+    name='exec',
+    help='Joins the interactive shell of running airflow container'
+    )
+@option_verbose
+@option_dry_run
[email protected]('exec_args', nargs=-1, type=click.UNPROCESSED)
+def exec(verbose: bool, dry_run: bool, exec_args: Tuple):

Review Comment:
   Docstring is needed here to be displayed in help.



##########
dev/breeze/src/airflow_breeze/shell/enter_shell.py:
##########
@@ -206,3 +206,31 @@ def enter_shell(**kwargs):
         console.print(CHEATSHEET, style=CHEATSHEET_STYLE)
     enter_shell_params = ShellParams(**filter_out_none(**updated_kwargs))
     run_shell_with_build_image_checks(verbose, dry_run, enter_shell_params)
+
+
+def find_airflow_container(**kwargs):
+    verbose = kwargs['verbose']
+    dry_run = kwargs['dry_run']
+    updated_kwargs = synchronize_cached_params(kwargs)

Review Comment:
   No need to do that - synchronize is only needed for "python", "backend", 
"mysql_version" etc. - all those parameters that are actually kept in cache 
(and I have an idea how to get rid of the synchronize method altogether - I 
will do it right after I merge the big PR of mine for build images.



##########
dev/breeze/src/airflow_breeze/global_constants.py:
##########
@@ -33,6 +33,8 @@
 DEFAULT_PYTHON_MAJOR_MINOR_VERSION = '3.7'
 DEFAULT_BACKEND = 'sqlite'
 
+DOCKER_COMPOSE_RUN_SCRIPT_FOR_CI = "dc_ci"

Review Comment:
   We can rmove that



##########
dev/breeze/src/airflow_breeze/shell/enter_shell.py:
##########
@@ -206,3 +206,31 @@ def enter_shell(**kwargs):
         console.print(CHEATSHEET, style=CHEATSHEET_STYLE)
     enter_shell_params = ShellParams(**filter_out_none(**updated_kwargs))
     run_shell_with_build_image_checks(verbose, dry_run, enter_shell_params)
+
+
+def find_airflow_container(**kwargs):

Review Comment:
   Better to pass the `verbose` and `dry_run` explicitly as parameters. no need 
to go via kwargs.



##########
dev/breeze/src/airflow_breeze/breeze.py:
##########
@@ -265,6 +265,9 @@
                 ],
             },
         ],
+        "breeze exec": [

Review Comment:
   I tihnk this logically goes after `start-airflow` and before `stop`  command 
so you should move it up a bit.



-- 
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