Skip to content

Commit 55f8a63

Browse files
authored
Make dry_run, verbose, answer breeze flags static (#27191)
The "dry_run" and "verbose" parameters had to be passed from the options passed to top-level breeze commands down to run_command and few other places where it was used. That was a lot of boilerplate and repeated identical declarations in multiple functions/methods. This change modifies those parameters so that the flag does noti have to be passed but it can be retrieved via global functions. At the same time those options have 'expose_value' set to False which make them not passed down that @click.command decorated functions as function arguments (and the same change is applied to `answer` option that has already been saved in static variable The --help output was reviewed and standardized across multiple commands.
1 parent 426f527 commit 55f8a63

File tree

71 files changed

+2978
-3373
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+2978
-3373
lines changed

dev/assign_cherry_picked_prs_with_milestone.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ def cli():
109109
"--dry-run",
110110
is_flag=True,
111111
help="Do not make any changes, just show what would have been done",
112+
envvar="DRY_RUN",
112113
)
113114

114115
option_skip_assigned = click.option(

dev/breeze/src/airflow_breeze/commands/ci_commands.py

Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
option_airflow_constraints_reference,
4444
option_answer,
4545
option_dry_run,
46-
option_github_repository,
4746
option_max_age,
4847
option_python,
4948
option_timezone,
@@ -73,39 +72,33 @@ def ci_group():
7372
@option_verbose
7473
@option_dry_run
7574
@option_answer
76-
def free_space(verbose: bool, dry_run: bool, answer: str):
75+
def free_space():
7776
if user_confirm("Are you sure to run free-space and perform cleanup?") == Answer.YES:
78-
run_command(["sudo", "swapoff", "-a"], verbose=verbose, dry_run=dry_run)
79-
run_command(["sudo", "rm", "-f", "/swapfile"], verbose=verbose, dry_run=dry_run)
77+
run_command(["sudo", "swapoff", "-a"])
78+
run_command(["sudo", "rm", "-f", "/swapfile"])
8079
for file in Path(tempfile.gettempdir()).iterdir():
8180
if file.name.startswith("parallel"):
8281
run_command(
8382
["sudo", "rm", "-rvf", os.fspath(file)],
84-
verbose=verbose,
85-
dry_run=dry_run,
8683
check=False,
8784
title=f"rm -rvf {file}",
8885
)
89-
run_command(["sudo", "apt-get", "clean"], verbose=verbose, dry_run=dry_run, check=False)
90-
run_command(
91-
["docker", "system", "prune", "--all", "--force", "--volumes"], verbose=verbose, dry_run=dry_run
92-
)
93-
run_command(["df", "-h"], verbose=verbose, dry_run=dry_run)
94-
run_command(["docker", "logout", "ghcr.io"], verbose=verbose, dry_run=dry_run, check=False)
86+
run_command(["sudo", "apt-get", "clean"], check=False)
87+
run_command(["docker", "system", "prune", "--all", "--force", "--volumes"])
88+
run_command(["df", "-h"])
89+
run_command(["docker", "logout", "ghcr.io"], check=False)
9590
run_command(
9691
["sudo", "rm", "-f", os.fspath(Path.home() / MSSQL_TMP_DIR_NAME)],
97-
verbose=verbose,
98-
dry_run=dry_run,
9992
)
10093

10194

10295
@ci_group.command(name="resource-check", help="Check if available docker resources are enough.")
10396
@option_verbose
10497
@option_dry_run
105-
def resource_check(verbose: bool, dry_run: bool):
106-
perform_environment_checks(verbose=verbose)
107-
shell_params = ShellParams(verbose=verbose, python=DEFAULT_PYTHON_MAJOR_MINOR_VERSION)
108-
check_docker_resources(shell_params.airflow_image_name, verbose=verbose, dry_run=dry_run)
98+
def resource_check():
99+
perform_environment_checks()
100+
shell_params = ShellParams(python=DEFAULT_PYTHON_MAJOR_MINOR_VERSION)
101+
check_docker_resources(shell_params.airflow_image_name)
109102

110103

111104
HOME_DIR = Path(os.path.expanduser("~")).resolve()
@@ -120,32 +113,30 @@ def resource_check(verbose: bool, dry_run: bool):
120113
]
121114

122115

123-
def fix_ownership_for_file(file: Path, dry_run: bool, verbose: bool):
116+
def fix_ownership_for_file(file: Path):
124117
get_console().print(f"[info]Fixing ownership of {file}")
125118
result = run_command(
126119
["sudo", "chown", f"{os.getuid}:{os.getgid()}", str(file.resolve())],
127120
check=False,
128121
stderr=subprocess.STDOUT,
129-
dry_run=dry_run,
130-
verbose=verbose,
131122
)
132123
if result.returncode != 0:
133124
get_console().print(f"[warning]Could not fix ownership for {file}: {result.stdout}")
134125

135126

136-
def fix_ownership_for_path(path: Path, dry_run: bool, verbose: bool):
127+
def fix_ownership_for_path(path: Path):
137128
if path.is_dir():
138129
for p in Path(path).rglob("*"):
139130
if p.owner == "root":
140-
fix_ownership_for_file(p, dry_run=dry_run, verbose=verbose)
131+
fix_ownership_for_file(p)
141132
else:
142133
if path.owner == "root":
143-
fix_ownership_for_file(path, dry_run=dry_run, verbose=verbose)
134+
fix_ownership_for_file(path)
144135

145136

146-
def fix_ownership_without_docker(dry_run: bool, verbose: bool):
137+
def fix_ownership_without_docker():
147138
for directory_to_fix in DIRECTORIES_TO_FIX:
148-
fix_ownership_for_path(directory_to_fix, dry_run=dry_run, verbose=verbose)
139+
fix_ownership_for_path(directory_to_fix)
149140

150141

151142
@ci_group.command(name="fix-ownership", help="Fix ownership of source files to be same as host user.")
@@ -155,10 +146,9 @@ def fix_ownership_without_docker(dry_run: bool, verbose: bool):
155146
help="Use sudo instead of docker image to fix the ownership. You need to be a `sudoer` to run it",
156147
envvar="USE_SUDO",
157148
)
158-
@option_github_repository
159149
@option_verbose
160150
@option_dry_run
161-
def fix_ownership(github_repository: str, use_sudo: bool, verbose: bool, dry_run: bool):
151+
def fix_ownership(use_sudo: bool):
162152
system = platform.system().lower()
163153
if system != "linux":
164154
get_console().print(
@@ -167,15 +157,15 @@ def fix_ownership(github_repository: str, use_sudo: bool, verbose: bool, dry_run
167157
sys.exit(0)
168158
if use_sudo:
169159
get_console().print("[info]Fixing ownership using sudo.")
170-
fix_ownership_without_docker(dry_run=dry_run, verbose=verbose)
160+
fix_ownership_without_docker()
171161
sys.exit(0)
172162
get_console().print("[info]Fixing ownership using docker.")
173-
fix_ownership_using_docker(dry_run=dry_run, verbose=verbose)
163+
fix_ownership_using_docker()
174164
# Always succeed
175165
sys.exit(0)
176166

177167

178-
def get_changed_files(commit_ref: str | None, dry_run: bool, verbose: bool) -> tuple[str, ...]:
168+
def get_changed_files(commit_ref: str | None) -> tuple[str, ...]:
179169
if commit_ref is None:
180170
return ()
181171
cmd = [
@@ -187,7 +177,7 @@ def get_changed_files(commit_ref: str | None, dry_run: bool, verbose: bool) -> t
187177
commit_ref + "^",
188178
commit_ref,
189179
]
190-
result = run_command(cmd, dry_run=dry_run, verbose=verbose, check=False, capture_output=True, text=True)
180+
result = run_command(cmd, check=False, capture_output=True, text=True)
191181
if result.returncode != 0:
192182
get_console().print(
193183
f"[warning] Error when running diff-tree command [/]\n{result.stdout}\n{result.stderr}"
@@ -244,14 +234,12 @@ def selective_check(
244234
default_branch: str,
245235
default_constraints_branch: str,
246236
github_event_name: str,
247-
verbose: bool,
248-
dry_run: bool,
249237
):
250238
from airflow_breeze.utils.selective_checks import SelectiveChecks
251239

252240
github_event = GithubEvents(github_event_name)
253241
if commit_ref is not None:
254-
changed_files = get_changed_files(commit_ref=commit_ref, dry_run=dry_run, verbose=verbose)
242+
changed_files = get_changed_files(commit_ref=commit_ref)
255243
else:
256244
changed_files = ()
257245
sc = SelectiveChecks(

0 commit comments

Comments
 (0)