This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch test-failing-composite-action in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 07c5312b51829a1ef17abfd0eb0d6eb695f2a7a3 Author: Jarek Potiuk <[email protected]> AuthorDate: Mon Oct 30 21:31:12 2023 +0100 Produce proper exit status in case image has been built with timeout When we build the image with timeout, we fork the process and set alarm and create a new process group in order to be sure that all the parallel build processes can be killed easily with sending termination signal to process group on timeout. In order to wait for the forked process to complete we used waitpid, but we did not handle the status code properly, so if the build failed, we returned with 0 exit code. This had the side effect that "Build CI image" did not fail, instead the next step (generating source providers failed instead and it was not obvious that the CI image build failing was the root cause. This PR properly retrieves the wait status and converts it to exit code - since we are still supporting Python 3.8 this is still done using a bit nasty set of if statements - only in Python 3.9 we have `os.waitstatus_to_exitcode` method to do it for us, but we cannot use the method yet. --- .../airflow_breeze/commands/ci_image_commands.py | 23 ++++++++++++++++++++-- dev/breeze/src/airflow_breeze/utils/parallel.py | 18 ++++++++--------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/dev/breeze/src/airflow_breeze/commands/ci_image_commands.py b/dev/breeze/src/airflow_breeze/commands/ci_image_commands.py index f5ab0b2d0f..cd7c915d8b 100644 --- a/dev/breeze/src/airflow_breeze/commands/ci_image_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/ci_image_commands.py @@ -206,6 +206,20 @@ def kill_process_group(build_process_group_id: int): pass +def get_exitcode(status: int) -> int: + # In Python 3.9+ we will be able to use + # os.waitstatus_to_exitcode(status) - see https://github.com/python/cpython/issues/84275 + # but until then we need to do this ugly conversion + if os.WIFSIGNALED(status): + return -os.WTERMSIG(status) + elif os.WIFEXITED(status): + return os.WEXITSTATUS(status) + elif os.WIFSTOPPED(status): + return -os.WSTOPSIG(status) + else: + return 1 + + @ci_image.command(name="build") @option_python @option_run_in_parallel @@ -277,8 +291,13 @@ def build( atexit.register(kill_process_group, pid) signal.signal(signal.SIGALRM, handler) signal.alarm(build_timeout_minutes * 60) - os.waitpid(pid, 0) - return + child_pid, status = os.waitpid(pid, 0) + exit_code = get_exitcode(status) + if exit_code: + get_console().print(f"[error]Exiting with exit code {exit_code}") + else: + get_console().print(f"[success]Exiting with exit code {exit_code}") + sys.exit(exit_code) else: # turn us into a process group leader os.setpgid(0, 0) diff --git a/dev/breeze/src/airflow_breeze/utils/parallel.py b/dev/breeze/src/airflow_breeze/utils/parallel.py index ea4ef06030..80415cc0e4 100644 --- a/dev/breeze/src/airflow_breeze/utils/parallel.py +++ b/dev/breeze/src/airflow_breeze/utils/parallel.py @@ -440,16 +440,14 @@ def check_async_run_results( get_console().print(f"\n[info]Summary: {outputs[i].escaped_title:<30}:\n") if print_lines: print(line) - try: - if errors: - get_console().print("\n[error]There were errors when running some tasks. Quitting.[/]\n") - sys.exit(1) - else: - get_console().print(f"\n[success]{success}[/]\n") - finally: - if not skip_cleanup: - for output in outputs: - Path(output.file_name).unlink(missing_ok=True) + if not skip_cleanup: + for output in outputs: + Path(output.file_name).unlink(missing_ok=True) + if errors: + get_console().print("\n[error]There were errors when running some tasks. Quitting.[/]\n") + sys.exit(1) + else: + get_console().print(f"\n[success]{success}[/]\n") @contextmanager
