You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by je...@apache.org on 2022/12/21 06:17:41 UTC
[airflow] branch main updated: Fix selective checks handling error tracebacks in CI (#28514)
This is an automated email from the ASF dual-hosted git repository.
jedcunningham pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new bc7feda66e Fix selective checks handling error tracebacks in CI (#28514)
bc7feda66e is described below
commit bc7feda66ed7bb2f2940fa90ef26ff90dd7a8c80
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Wed Dec 21 07:17:19 2022 +0100
Fix selective checks handling error tracebacks in CI (#28514)
Initially selective check was implemented in the way that it printed
diagnostic output on stdout and the GITHUB_OUTPUT compatible set of
outputs on stderr so that it could be redirected to the GITHUB_OUTPUT
in its entirety. But this turned out to be a bad idea because when
there was an error generated in selective-checks themselves, the
traceback was printed in stderr and redirecting stderr to GITHUB_OUTPUT
swallowed the traceback.
This change reverses the behaviour:
* diagnostic output is printed to stderr
* GITHUB_OUTPUT compatible output is printed to stdout
This way when traceback happens it is printed to stderr and is not
swalleowed by redirection to GITHUB_OUTPUT
---
.github/workflows/ci.yml | 3 +-
.../src/airflow_breeze/commands/ci_commands.py | 12 ++++----
.../src/airflow_breeze/utils/github_actions.py | 4 +--
.../src/airflow_breeze/utils/selective_checks.py | 36 +++++++++++-----------
4 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 7cf54ccb7b..83d55acfea 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -221,7 +221,8 @@ jobs:
env:
PR_LABELS: "${{ steps.source-run-info.outputs.pr-labels }}"
COMMIT_REF: "${{ github.sha }}"
- run: breeze ci selective-check 2>> ${GITHUB_OUTPUT}
+ VERBOSE: "false"
+ run: breeze ci selective-check >> ${GITHUB_OUTPUT}
- name: env
run: printenv
env:
diff --git a/dev/breeze/src/airflow_breeze/commands/ci_commands.py b/dev/breeze/src/airflow_breeze/commands/ci_commands.py
index 39de15a76a..e62875630b 100644
--- a/dev/breeze/src/airflow_breeze/commands/ci_commands.py
+++ b/dev/breeze/src/airflow_breeze/commands/ci_commands.py
@@ -50,7 +50,7 @@ from airflow_breeze.utils.common_options import (
option_verbose,
)
from airflow_breeze.utils.confirm import Answer, user_confirm
-from airflow_breeze.utils.console import get_console
+from airflow_breeze.utils.console import get_console, get_stderr_console
from airflow_breeze.utils.custom_param_types import BetterChoice
from airflow_breeze.utils.docker_command_utils import (
check_docker_resources,
@@ -179,14 +179,14 @@ def get_changed_files(commit_ref: str | None) -> tuple[str, ...]:
]
result = run_command(cmd, check=False, capture_output=True, text=True)
if result.returncode != 0:
- get_console().print(
+ get_stderr_console().print(
f"[warning] Error when running diff-tree command [/]\n{result.stdout}\n{result.stderr}"
)
return ()
changed_files = tuple(result.stdout.splitlines()) if result.stdout else ()
- get_console().print("\n[info]Changed files:[/]\n")
- get_console().print(changed_files)
- get_console().print()
+ get_stderr_console().print("\n[info]Changed files:[/]\n")
+ get_stderr_console().print(changed_files)
+ get_stderr_console().print()
return changed_files
@@ -250,7 +250,7 @@ def selective_check(
pr_labels=tuple(ast.literal_eval(pr_labels)) if pr_labels else (),
github_event=github_event,
)
- print(str(sc), file=sys.stderr)
+ print(str(sc), file=sys.stdout)
@ci_group.command(name="find-newer-dependencies", help="Finds which dependencies are being upgraded.")
diff --git a/dev/breeze/src/airflow_breeze/utils/github_actions.py b/dev/breeze/src/airflow_breeze/utils/github_actions.py
index 6b8043aa7e..1566bbe81c 100644
--- a/dev/breeze/src/airflow_breeze/utils/github_actions.py
+++ b/dev/breeze/src/airflow_breeze/utils/github_actions.py
@@ -20,11 +20,11 @@ from typing import Any
from rich.markup import escape
-from airflow_breeze.utils.console import get_console
+from airflow_breeze.utils.console import get_stderr_console
def get_ga_output(name: str, value: Any) -> str:
output_name = name.replace("_", "-")
printed_value = str(value).lower() if isinstance(value, bool) else value
- get_console().print(f"[info]{output_name}[/] = [green]{escape(str(printed_value))}[/]")
+ get_stderr_console().print(f"[info]{output_name}[/] = [green]{escape(str(printed_value))}[/]")
return f"{output_name}={printed_value}"
diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
index eeaf65150e..d978434b3c 100644
--- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py
+++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
@@ -54,7 +54,7 @@ from airflow_breeze.global_constants import (
SelectiveUnitTestTypes,
all_selective_test_types,
)
-from airflow_breeze.utils.console import get_console
+from airflow_breeze.utils.console import get_stderr_console
FULL_TESTS_NEEDED_LABEL = "full tests needed"
DEBUG_CI_RESOURCES_LABEL = "debug ci resources"
@@ -295,16 +295,16 @@ class SelectiveChecks:
@cached_property
def full_tests_needed(self) -> bool:
if not self._commit_ref:
- get_console().print("[warning]Running everything as commit is missing[/]")
+ get_stderr_console().print("[warning]Running everything as commit is missing[/]")
return True
if self._github_event in [GithubEvents.PUSH, GithubEvents.SCHEDULE, GithubEvents.WORKFLOW_DISPATCH]:
- get_console().print(f"[warning]Full tests needed because event is {self._github_event}[/]")
+ get_stderr_console().print(f"[warning]Full tests needed because event is {self._github_event}[/]")
return True
if len(self._matching_files(FileGroupForCi.ENVIRONMENT_FILES, CI_FILE_GROUP_MATCHES)) > 0:
- get_console().print("[warning]Running everything because env files changed[/]")
+ get_stderr_console().print("[warning]Running everything because env files changed[/]")
return True
if FULL_TESTS_NEEDED_LABEL in self._pr_labels:
- get_console().print(
+ get_stderr_console().print(
"[warning]Full tests needed because "
f"label '{FULL_TESTS_NEEDED_LABEL}' is in {self._pr_labels}[/]"
)
@@ -434,24 +434,24 @@ class SelectiveChecks:
self._match_files_with_regexps(matched_files, regexps)
count = len(matched_files)
if count > 0:
- get_console().print(f"[warning]{match_group} matched {count} files.[/]")
- get_console().print(matched_files)
+ get_stderr_console().print(f"[warning]{match_group} matched {count} files.[/]")
+ get_stderr_console().print(matched_files)
else:
- get_console().print(f"[warning]{match_group} did not match any file.[/]")
+ get_stderr_console().print(f"[warning]{match_group} did not match any file.[/]")
return matched_files
def _should_be_run(self, source_area: FileGroupForCi) -> bool:
if self.full_tests_needed:
- get_console().print(f"[warning]{source_area} enabled because we are running everything[/]")
+ get_stderr_console().print(f"[warning]{source_area} enabled because we are running everything[/]")
return True
matched_files = self._matching_files(source_area, CI_FILE_GROUP_MATCHES)
if len(matched_files) > 0:
- get_console().print(
+ get_stderr_console().print(
f"[warning]{source_area} enabled because it matched {len(matched_files)} changed files[/]"
)
return True
else:
- get_console().print(
+ get_stderr_console().print(
f"[warning]{source_area} disabled because it did not match any changed files[/]"
)
return False
@@ -503,7 +503,7 @@ class SelectiveChecks:
count = len(matched_files)
if count > 0:
test_types.add(test_type.value)
- get_console().print(f"[warning]{test_type} added because it matched {count} files[/]")
+ get_stderr_console().print(f"[warning]{test_type} added because it matched {count} files[/]")
return matched_files
def _get_test_types_to_run(self) -> list[str]:
@@ -528,11 +528,11 @@ class SelectiveChecks:
remaining_files = set(all_source_files) - set(matched_files) - set(kubernetes_files)
count_remaining_files = len(remaining_files)
if count_remaining_files > 0:
- get_console().print(
+ get_stderr_console().print(
f"[warning]We should run all tests. There are {count_remaining_files} changed "
"files that seems to fall into Core/Other category[/]"
)
- get_console().print(remaining_files)
+ get_stderr_console().print(remaining_files)
candidate_test_types.update(all_selective_test_types())
else:
if "Providers" in candidate_test_types:
@@ -540,12 +540,12 @@ class SelectiveChecks:
if len(affected_providers) != 0:
candidate_test_types.remove("Providers")
candidate_test_types.add(f"Providers[{','.join(sorted(affected_providers))}]")
- get_console().print(
+ get_stderr_console().print(
"[warning]There are no core/other files. Only tests relevant to the changed files are run.[/]"
)
sorted_candidate_test_types = list(sorted(candidate_test_types))
- get_console().print("[warning]Selected test type candidates to run:[/]")
- get_console().print(sorted_candidate_test_types)
+ get_stderr_console().print("[warning]Selected test type candidates to run:[/]")
+ get_stderr_console().print(sorted_candidate_test_types)
return sorted_candidate_test_types
@cached_property
@@ -560,7 +560,7 @@ class SelectiveChecks:
test_types_to_remove: set[str] = set()
for test_type in current_test_types:
if test_type.startswith("Providers"):
- get_console().print(
+ get_stderr_console().print(
f"[warning]Removing {test_type} because the target branch "
f"is {self._default_branch} and not main[/]"
)