From 59627e27689cb23103700c4a7c91c08589330e31 Mon Sep 17 00:00:00 2001 From: Yaswant Pradhan <2984440+yaswant@users.noreply.github.com> Date: Wed, 14 Jan 2026 16:51:40 +0000 Subject: [PATCH 1/2] Refactor run_command error handling --- github_scripts/get_git_sources.py | 54 ++++++++++++++-------- github_scripts/rose_stem_extract_source.py | 34 ++++++++------ 2 files changed, 56 insertions(+), 32 deletions(-) diff --git a/github_scripts/get_git_sources.py b/github_scripts/get_git_sources.py index d5ec8d1..af5966c 100644 --- a/github_scripts/get_git_sources.py +++ b/github_scripts/get_git_sources.py @@ -1,8 +1,9 @@ -# *****************************COPYRIGHT******************************* +# ----------------------------------------------------------------------------- # (C) Crown copyright Met Office. All rights reserved. -# For further details please refer to the file COPYRIGHT.txt -# which you should have received as part of this distribution. -# *****************************COPYRIGHT******************************* +# The file LICENCE, distributed with this code, contains details of the terms +# under which the code may be used. +# ----------------------------------------------------------------------------- + """ Clone sources for a rose-stem run for use with git bdiff module in scripts """ @@ -13,10 +14,16 @@ from pathlib import Path from shutil import rmtree import shlex +import logging + +logger = logging.getLogger(__name__) def run_command( - command: str, rval: bool = False, check: bool = True + command: str, + check: bool = True, + capture: bool = True, + timeout: int = 300 ) -> Optional[subprocess.CompletedProcess]: """ Run a subprocess command and return the result object @@ -25,23 +32,32 @@ def run_command( Outputs: - result object from subprocess.run """ - command = shlex.split(command) - result = subprocess.run( - command, - capture_output=True, - text=True, - timeout=300, - shell=False, - check=False, - ) - if check and result.returncode: - print(result.stdout, end="\n\n\n") - raise RuntimeError( - f"[FAIL] Issue found running command {command}\n\n{result.stderr}" + + args = shlex.split(command) + + try: + # Note: text=True and capture_output=True have high overhead + # for large buffers. Use capture=False for fire-and-forget tasks. + result = subprocess.run( + args, + capture_output=capture, + text=capture, + timeout=timeout, + shell=False, + check=False ) - if rval: + if check and result.returncode != 0: + err_msg = (result.stderr or "").strip() + logger.error(f"[FAIL] Command failed: {command}\nError: {err_msg}") + raise subprocess.CalledProcessError( + result.returncode, args, output=result.stdout, stderr=result.stderr + ) return result + except (subprocess.TimeoutExpired, FileNotFoundError) as e: + logger.error(f"[FAIL] Execution error for '{args[0]}': {e}") + raise + def clone_repo_mirror( source: str, repo_ref: str, parent: str, mirror_loc: Path, loc: Path diff --git a/github_scripts/rose_stem_extract_source.py b/github_scripts/rose_stem_extract_source.py index 05fa081..028d331 100755 --- a/github_scripts/rose_stem_extract_source.py +++ b/github_scripts/rose_stem_extract_source.py @@ -1,9 +1,10 @@ #!/usr/bin/env python3 -# *****************************COPYRIGHT******************************* +# ----------------------------------------------------------------------------- # (C) Crown copyright Met Office. All rights reserved. -# For further details please refer to the file COPYRIGHT.txt -# which you should have received as part of this distribution. -# *****************************COPYRIGHT******************************* +# The file LICENCE, distributed with this code, contains details of the terms +# under which the code may be used. +# ----------------------------------------------------------------------------- + """ Clone sources for a rose-stem run for use with git bdiff module in scripts Only intended for use with rose-stem suites that have provided appropriate environment @@ -11,14 +12,12 @@ """ import os -from datetime import datetime from pathlib import Path from ast import literal_eval from get_git_sources import clone_repo, clone_repo_mirror, sync_repo -from typing import Dict -def set_https(dependencies: Dict) -> Dict: +def set_https(dependencies: dict) -> dict: """ Change sources in a dependencies dictions to use https instead of ssh """ @@ -35,32 +34,41 @@ def set_https(dependencies: Dict) -> Dict: def main() -> None: + """ + 1. Read environment variables for: + SOURCE_DIRECTORY - location to clone sources, + DEPENDENCIES - dictionary of dependencies, + USE_TOKENS - whether to use tokens for https URLs, + USE_MIRRORS - whether to use local git mirrors, + GIT_MIRROR_LOC - location of local git mirrors + 2. For each dependency in DEPENDENCIES, clone or sync the source + 3. If USE_TOKENS is True, modify the source URLs to use https + 4. If USE_MIRRORS is True, clone from local mirrors at GIT_MIRROR_LOC + """ clone_loc = Path(os.environ["SOURCE_DIRECTORY"]) - dependencies: Dict = literal_eval(os.environ["DEPENDENCIES"]) + dependencies: dict = literal_eval(os.environ["DEPENDENCIES"]) if os.environ.get("USE_TOKENS", "False") == "True": dependencies = set_https(dependencies) for dependency, values in dependencies.items(): - print( - f"Extracting {dependency} at time {datetime.now()} " - f"using source {values['source']} and ref {values['ref']}" - ) - loc = clone_loc / dependency if ".git" in values["source"]: if os.environ.get("USE_MIRRORS", "False") == "True": mirror_loc = Path(os.environ["GIT_MIRROR_LOC"]) / values["parent"] + print(f"Cloning {dependency} from {mirror_loc} at ref {values['ref']}") clone_repo_mirror( values["source"], values["ref"], values["parent"], mirror_loc, loc ) else: + print(f"Cloning {dependency} from {values['source']} at ref {values['ref']}") clone_repo(values["source"], values["ref"], loc) else: + print(f"Syncing {dependency} at ref {values['ref']}") sync_repo(values["source"], values["ref"], loc) From c0b5d73045647aeb03c5bbb15e0f4811c36797ce Mon Sep 17 00:00:00 2001 From: Yaswant Pradhan <2984440+yaswant@users.noreply.github.com> Date: Thu, 15 Jan 2026 17:54:29 +0000 Subject: [PATCH 2/2] Add timestamps in rose_stem_extract_source.py --- github_scripts/rose_stem_extract_source.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/github_scripts/rose_stem_extract_source.py b/github_scripts/rose_stem_extract_source.py index 028d331..5a5e590 100755 --- a/github_scripts/rose_stem_extract_source.py +++ b/github_scripts/rose_stem_extract_source.py @@ -7,14 +7,15 @@ """ Clone sources for a rose-stem run for use with git bdiff module in scripts -Only intended for use with rose-stem suites that have provided appropriate environment -variables +Only intended for use with rose-stem suites that have provided appropriate +environment variables """ import os from pathlib import Path from ast import literal_eval from get_git_sources import clone_repo, clone_repo_mirror, sync_repo +from datetime import datetime def set_https(dependencies: dict) -> dict: @@ -47,28 +48,35 @@ def main() -> None: """ clone_loc = Path(os.environ["SOURCE_DIRECTORY"]) - dependencies: dict = literal_eval(os.environ["DEPENDENCIES"]) if os.environ.get("USE_TOKENS", "False") == "True": dependencies = set_https(dependencies) for dependency, values in dependencies.items(): - loc = clone_loc / dependency if ".git" in values["source"]: if os.environ.get("USE_MIRRORS", "False") == "True": mirror_loc = Path(os.environ["GIT_MIRROR_LOC"]) / values["parent"] - print(f"Cloning {dependency} from {mirror_loc} at ref {values['ref']}") + print( + f"[{datetime.now().strftime('%Y-%m-%d %H:%M:%S')}] Cloning " + f"{dependency} from {mirror_loc} at ref {values['ref']}" + ) clone_repo_mirror( values["source"], values["ref"], values["parent"], mirror_loc, loc ) else: - print(f"Cloning {dependency} from {values['source']} at ref {values['ref']}") + print( + f"[{datetime.now().strftime('%Y-%m-%d %H:%M:%S')}] Cloning " + f"{dependency} from {values['source']} at ref {values['ref']}" + ) clone_repo(values["source"], values["ref"], loc) else: - print(f"Syncing {dependency} at ref {values['ref']}") + print( + f"[{datetime.now().strftime('%Y-%m-%d %H:%M:%S')}] Syncing " + f"{dependency} at ref {values['ref']}" + ) sync_repo(values["source"], values["ref"], loc)