Skip to content

Commit

Permalink
Remove stderr redirection for get-workflow-info (#29381)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
potiuk authored Feb 6, 2023
1 parent 307377a commit 9f1f026
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 31 deletions.
1 change: 0 additions & 1 deletion .github/actions/get-target-branch-build-scripts/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release_dockerhub_image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}"
Expand Down
12 changes: 6 additions & 6 deletions dev/breeze/src/airflow_breeze/commands/ci_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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.")
Expand Down
4 changes: 2 additions & 2 deletions dev/breeze/src/airflow_breeze/utils/github_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
36 changes: 18 additions & 18 deletions dev/breeze/src/airflow_breeze/utils/selective_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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}[/]"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -534,24 +534,24 @@ 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:
affected_providers = find_all_providers_affected(changed_files=self._files)
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
Expand All @@ -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[/]"
)
Expand Down

0 comments on commit 9f1f026

Please sign in to comment.