From 9f1f026fac27a68c48c70a6e5b257b39f9396632 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Mon, 6 Feb 2023 15:58:48 +0100 Subject: [PATCH] Remove stderr redirection for get-workflow-info (#29381) The #28514 change added capability of getting trace errors for the command by swapping redirection of stderr and stdout, but debug output for github_output file remoined in stderr. This was no problem in most cases as rich did not produce color codes for stderr redirection and github_output was able to parse it properly - and each line in the GITHHUB_OUTPUT was simply duplicated. However, when number of labels was big, rich split the message and it caused Invalid format message. --- .../action.yml | 1 - .github/workflows/build-images.yml | 2 +- .github/workflows/ci.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/release_dockerhub_image.yml | 2 +- .../airflow_breeze/commands/ci_commands.py | 12 +++---- .../airflow_breeze/utils/github_actions.py | 4 +-- .../airflow_breeze/utils/selective_checks.py | 36 +++++++++---------- 8 files changed, 30 insertions(+), 31 deletions(-) diff --git a/.github/actions/get-target-branch-build-scripts/action.yml b/.github/actions/get-target-branch-build-scripts/action.yml index 3d32578a2f42d..6d0aac35d7f1e 100644 --- a/.github/actions/get-target-branch-build-scripts/action.yml +++ b/.github/actions/get-target-branch-build-scripts/action.yml @@ -17,7 +17,6 @@ # --- name: 'Gets target branch build scripts' -description: 'Checks out target branch build scripts (including breeze) and replaces the current version' runs: using: "composite" steps: diff --git a/.github/workflows/build-images.yml b/.github/workflows/build-images.yml index 78f8468493cc9..fa8686890d0fe 100644 --- a/.github/workflows/build-images.yml +++ b/.github/workflows/build-images.yml @@ -148,7 +148,7 @@ jobs: PR_LABELS: "${{ steps.get-latest-pr-labels.outputs.pull-request-labels }}" COMMIT_REF: "${{ env.TARGET_COMMIT_SHA }}" VERBOSE: "false" - run: breeze ci selective-check >> ${GITHUB_OUTPUT} + run: breeze ci selective-check 2>> ${GITHUB_OUTPUT} - name: env run: printenv env: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 415e39c1eccf0..23e795d3823d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -224,7 +224,7 @@ jobs: PR_LABELS: "${{ steps.source-run-info.outputs.pr-labels }}" COMMIT_REF: "${{ github.sha }}" VERBOSE: "false" - run: breeze ci selective-check >> ${GITHUB_OUTPUT} + run: breeze ci selective-check 2>> ${GITHUB_OUTPUT} - name: env run: printenv env: diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 858b8a5d6b5e1..cec9eaf9954b4 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -50,7 +50,7 @@ jobs: env: COMMIT_REF: "${{ github.sha }}" VERBOSE: "false" - run: breeze ci selective-check >> ${GITHUB_OUTPUT} + run: breeze ci selective-check 2>> ${GITHUB_OUTPUT} analyze: name: Analyze diff --git a/.github/workflows/release_dockerhub_image.yml b/.github/workflows/release_dockerhub_image.yml index c2afcd67dd1a2..fca33a204e794 100644 --- a/.github/workflows/release_dockerhub_image.yml +++ b/.github/workflows/release_dockerhub_image.yml @@ -59,7 +59,7 @@ jobs: id: selective-checks env: VERBOSE: "false" - run: breeze ci selective-check >> ${GITHUB_OUTPUT} + run: breeze ci selective-check 2>> ${GITHUB_OUTPUT} release-images: timeout-minutes: 120 name: "Release images: ${{ github.event.inputs.airflowVersion }}, ${{ matrix.python-version }}" diff --git a/dev/breeze/src/airflow_breeze/commands/ci_commands.py b/dev/breeze/src/airflow_breeze/commands/ci_commands.py index e62875630be7f..39de15a76a08a 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 @@ option_verbose, ) from airflow_breeze.utils.confirm import Answer, user_confirm -from airflow_breeze.utils.console import get_console, get_stderr_console +from airflow_breeze.utils.console import get_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_stderr_console().print( + get_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_stderr_console().print("\n[info]Changed files:[/]\n") - get_stderr_console().print(changed_files) - get_stderr_console().print() + get_console().print("\n[info]Changed files:[/]\n") + get_console().print(changed_files) + get_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.stdout) + print(str(sc), file=sys.stderr) @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 1566bbe81c8d6..6b8043aa7e726 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 rich.markup import escape -from airflow_breeze.utils.console import get_stderr_console +from airflow_breeze.utils.console import get_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_stderr_console().print(f"[info]{output_name}[/] = [green]{escape(str(printed_value))}[/]") + get_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 8c2b4ac87259c..75fb5f0faa387 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 @@ SelectiveUnitTestTypes, all_selective_test_types, ) -from airflow_breeze.utils.console import get_stderr_console +from airflow_breeze.utils.console import get_console FULL_TESTS_NEEDED_LABEL = "full tests needed" DEBUG_CI_RESOURCES_LABEL = "debug ci resources" @@ -298,16 +298,16 @@ def default_constraints_branch(self) -> str: @cached_property def full_tests_needed(self) -> bool: if not self._commit_ref: - get_stderr_console().print("[warning]Running everything as commit is missing[/]") + get_console().print("[warning]Running everything as commit is missing[/]") return True if self._github_event in [GithubEvents.PUSH, GithubEvents.SCHEDULE, GithubEvents.WORKFLOW_DISPATCH]: - get_stderr_console().print(f"[warning]Full tests needed because event is {self._github_event}[/]") + get_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_stderr_console().print("[warning]Running everything because env files changed[/]") + get_console().print("[warning]Running everything because env files changed[/]") return True if FULL_TESTS_NEEDED_LABEL in self._pr_labels: - get_stderr_console().print( + get_console().print( "[warning]Full tests needed because " f"label '{FULL_TESTS_NEEDED_LABEL}' is in {self._pr_labels}[/]" ) @@ -437,24 +437,24 @@ def _matching_files(self, match_group: T, match_dict: dict[T, list[str]]) -> lis self._match_files_with_regexps(matched_files, regexps) count = len(matched_files) if count > 0: - get_stderr_console().print(f"[warning]{match_group} matched {count} files.[/]") - get_stderr_console().print(matched_files) + get_console().print(f"[warning]{match_group} matched {count} files.[/]") + get_console().print(matched_files) else: - get_stderr_console().print(f"[warning]{match_group} did not match any file.[/]") + get_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_stderr_console().print(f"[warning]{source_area} enabled because we are running everything[/]") + get_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_stderr_console().print( + get_console().print( f"[warning]{source_area} enabled because it matched {len(matched_files)} changed files[/]" ) return True else: - get_stderr_console().print( + get_console().print( f"[warning]{source_area} disabled because it did not match any changed files[/]" ) return False @@ -506,7 +506,7 @@ def _select_test_type_if_matching( count = len(matched_files) if count > 0: test_types.add(test_type.value) - get_stderr_console().print(f"[warning]{test_type} added because it matched {count} files[/]") + get_console().print(f"[warning]{test_type} added because it matched {count} files[/]") return matched_files def _get_test_types_to_run(self) -> list[str]: @@ -534,11 +534,11 @@ def _get_test_types_to_run(self) -> list[str]: ) count_remaining_files = len(remaining_files) if count_remaining_files > 0: - get_stderr_console().print( + get_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_stderr_console().print(remaining_files) + get_console().print(remaining_files) candidate_test_types.update(all_selective_test_types()) else: if "Providers" in candidate_test_types: @@ -546,12 +546,12 @@ def _get_test_types_to_run(self) -> list[str]: if len(affected_providers) != 0: candidate_test_types.remove("Providers") candidate_test_types.add(f"Providers[{','.join(sorted(affected_providers))}]") - get_stderr_console().print( + get_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_stderr_console().print("[warning]Selected test type candidates to run:[/]") - get_stderr_console().print(sorted_candidate_test_types) + get_console().print("[warning]Selected test type candidates to run:[/]") + get_console().print(sorted_candidate_test_types) return sorted_candidate_test_types @cached_property @@ -566,7 +566,7 @@ def test_types(self) -> str: test_types_to_remove: set[str] = set() for test_type in current_test_types: if test_type.startswith("Providers"): - get_stderr_console().print( + get_console().print( f"[warning]Removing {test_type} because the target branch " f"is {self._default_branch} and not main[/]" )