Breeze adding exec command

[Bowrna]

closes: #21104

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

[Bowrna]

@potiuk Could you explain to me what will happen in this command? I tried executing docker-compose ps directly via terminal but it throws the following error.

ERROR:
        Can't find a suitable configuration file in this directory or any
        parent. Are you in the right directory?

        Supported filenames: docker-compose.yml, docker-compose.yaml, compose.yml, compose.yaml

But when executed via the dc_ci file in .build folder it works.

airflow/breeze-legacy

Lines 3512 to 3513 in 3a2eb96

	airflow_testing_container=$("${dc_run_file}" ps | grep airflow | awk '{print $1}' 2>/dev/null)
	: "${airflow_testing_container:?"ERROR! Breeze must be running in order to exec into running container"}"

Do I have to execute the "${dc_run_file}" ps via subprocess to get the process id?

[joppevos]

@potiuk Could you explain to me what will happen in this command? I tried executing docker-compose ps directly via terminal but it throws the following error.

docker-compose ps lists the containers status in a nicely formatted way. The naming probably comes from the command ps which shows the process status

The error states that it cannot find any configuration file. Check that you are running it in the right location where it can discover the config.

[Bowrna]

@potiuk Could you explain to me what will happen in this command? I tried executing docker-compose ps directly via terminal but it throws the following error.

docker-compose ps lists the containers status in a nicely formatted way. The naming probably comes from the command ps which shows the process status

The error states that it cannot find any configuration file. Check that you are running it in the right location where it can discover the config.

thanks @joppevos yes docker-compose ps works when executed via dc_run_file ps but it doesn't work when executed directly. I tried from airflow_home and airflow_home/.build folder. Let me check other paths too.

[potiuk]

We should really run docker-compose ps here. The bash attempted to use the generated dc_run file which is not needed any more,

But what we need we need exactly the same parameters that are used by ShellParams in shell commmand. Otherwise docker-compose does not know which file and which configuration should be used.

When we run docker-compose in Shell command we add 👍

• COMPOSE_FILE variable
• Some environment variables for PORTs etc.

only when we use the same parameters and COMPOSE_FILE that we used for "breeze shell" we will be able to find running processes with "docker-compose ps" command.

So in short:

• don't use dc_run (it's the way docker compose command was run in Breeze)
• run docker-compose same way as in "shell" command (with the same env variables etc).
• and be a bit smarter when finding containers. I have not used the right filterring when I run the command , but it can be done a bit "smarter" with --filter command to only filter running airflow container.
• in order to test all the filtering, it might actually be nice to implement docker-compose command from the old Breeze. This command started docker-compose (same way as Shell command) and passed all the extra arguments passed directly to docker-compose command - this might be nice way to test various filtering strings.

[Bowrna]

@potiuk Could you tell me how i can filter running airflow container? I can filter the running container, but specifically to get only the airflow container which property of the running container i could rely upon.

[potiuk]

Not sure entirely ;) . Docker and Docker compose filters are pretty weakly documented. But filter "give me container of the aiflow service" is what we are looking for. In the base.yml we have:

version: "3.7"
services:
  airflow:
    image: ${AIRFLOW_CI_IMAGE_WITH_TAG}

So our container for airflow will always run as "airflow" service.

[Bowrna]

I see :) I will check this. I will also take this as an opportunity to contribute to docs in Docker-compose. Thanks @potiuk

[potiuk]

@potiuk Could you explain to me what will happen in this command? I tried executing docker-compose ps directly via terminal but it throws the following error.

docker-compose ps lists the containers status in a nicely formatted way. The naming probably comes from the command ps which shows the process status
The error states that it cannot find any configuration file. Check that you are running it in the right location where it can discover the config.

thanks @joppevos yes docker-compose ps works when executed via dc_run_file ps but it doesn't work when executed directly. I tried from airflow_home and airflow_home/.build folder. Let me check other paths too.

That's becode dc_run_file from the old breeze already contains COMPOSE_FILE and all the environment variables that docker-compose needs to find out the definition of Breeze containers (see the other comment). This is precisely what we do in Shell command when we launch "docker-compose" - se we should use the same mechanism as in "shell" to get the variables and run docker-compose with all of them set.

[Bowrna]

@potiuk Do I need to make any modifications in this PR wrt your current changes?

[potiuk]

Some small corrections

[potiuk]

@potiuk Do I need to make any modifications in this PR wrt your current changes?

Not really - I think the only change that I applied that woudl affect this one is a little bit "nicer" handling of run_command results: when check = False but we actually care about the success/failure (for example when I run parallel runs). I think in this case just printing output is fine (I commented it) as exec is really 'interactive only' but it would be nice to catch the process return code and sys.exit() with it:

smth like:

process = run_command()
if not process:
   sys.exit(1)
sys,exit(process.returncode)

[Bowrna]

I will fix the changes that you have suggested @potiuk
I have added docker-compose filter option to get the container id

[potiuk]

I will fix the changes that you have suggested @potiuk I have added docker-compose filter option to get the container id

Yep. Looks much better with the filter :)

[Bowrna]

@potiuk Do i need to make bring any more changes to this command? Can you give your suggestion?

[potiuk]

I think the "exec" command has been "lost" during rebase :) - I removed all commands from the breeze.py so when you rebased, it has been removed during conflict resolution.

You need to re-add it :). I copied some changes from the previous version below:

                ],
            },
        ],
        "breeze exec": [
            {"name": "Drops in the interactive shell of active airflow container"},
        ],

@main.command(name='exec', help='Joins the interactive shell of running airflow container')
@option_verbose
@option_dry_run
@click.argument('exec_args', nargs=-1, type=click.UNPROCESSED)
def exec(verbose: bool, dry_run: bool, exec_args: Tuple):
    container_running = find_airflow_container(verbose, dry_run)
    cmd_to_run = [
        "docker",
        "exec",
	@@ -1510,14 +1507,17 @@ def exec(verbose: bool, dry_run: bool, exec_args: Tuple):

    if exec_args:
        cmd_to_run.extend(exec_args)
    process = run_command(
        cmd_to_run,
        verbose=verbose,
        dry_run=dry_run,
        check=False,
        no_output_dump_on_exception=False,
        text=True,
    )
    if not process:
        sys.exit(1)
    sys.exit(process.returncode)

[potiuk]

It should be added in https://github.com/apache/airflow/blob/main/dev/breeze/src/airflow_breeze/commands/developer_commands.py - that's where all regular "development" commands were moved.

[potiuk]

The breeze.py looks like this now :).

from airflow_breeze.commands.configure_rich_click import click  # noqa
from airflow_breeze.commands.main import main
from airflow_breeze.utils.path_utils import find_airflow_sources_root_to_operate_on

find_airflow_sources_root_to_operate_on()

if __name__ == '__main__':
    main()

[Bowrna]

@potiuk After rebasing the code, the code doesn't work properly. docker-compose ps doesn't list the airflow breeze container. I tried running with the old breeze and there issue arises due to the platform and sending the platform explicitly as linux/amd64 also doesn't fix the process. I tried running via the dc_ci script in .build folder and it doesn't list the active breeze container.

Could you help me to find the issue?

[potiuk]

I just tested it an well - it works well for me :)

[potiuk]

Just one static check error :)

[potiuk]

I thin you will need to rebase after I merged #23311

[github-actions]

The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main or amend the last commit of the PR, and push it with --force-with-lease.