From d9559c2a8d89c56f7d00b9dc80a9e400c38d8c86 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Tue, 18 Nov 2025 20:11:29 +0300 Subject: [PATCH 1/2] Adds Pyright static type checker --- .github/PULL_REQUEST_TEMPLATE.md | 6 +++--- doc/developers.md | 6 +++--- pipelines/controller-cli/README.md | 22 +++++++++++++++------- requirements-dev.txt | 1 + 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 9451e3cb7..0a1eb509a 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -40,9 +40,9 @@ automated e2e/unit tests. code that was well tested you do not have to add tests) - [ ] I ran `mvn clean package` right before creating this pull request and added all formatting changes to my commit. -- [ ] If I made any Python code changes, I ran `black .` and `pylint .` right - before creating this pull request and added all formatting changes to my - commit. +- [ ] If I made any Python code changes, I ran `black .`, `pylint .` and + `pyright` . right before creating this pull request and added all + formatting changes to my commit. - [ ] All new and existing **tests passed**. - [ ] My pull request is **based on the latest changes** of the master branch. diff --git a/doc/developers.md b/doc/developers.md index 46bf0445d..8254e6b70 100644 --- a/doc/developers.md +++ b/doc/developers.md @@ -50,9 +50,9 @@ replay the new events with no more interactions with OpenMRS. ## Python development -This requires you to run to install some dev dependencies e.g. `black` and -`pylint` packages. It is a good idea to first create a Python `virtualenv`: (Run -these commands from the root of the repo) +This requires you to run to install some dev dependencies e.g. `black`, `pylint` +and `pyright` packages. It is a good idea to first create a Python `virtualenv`: +(Run these commands from the root of the repo) _Note:_ we removed `isort` since now have `pylint` which can flag out of order imports that the developer can check and fix. diff --git a/pipelines/controller-cli/README.md b/pipelines/controller-cli/README.md index 04e2fee4f..1deca8abb 100644 --- a/pipelines/controller-cli/README.md +++ b/pipelines/controller-cli/README.md @@ -85,16 +85,24 @@ Then you can execute the following command from the project's root directory: ## Formatting and Linting -This project uses `black` for code formatting, and `PyLint` for linting. To run -these commands, you need to have installed the requirements in the repo's root -declared in the `requirements-dev.txt` file _(i.e. navigating to the root and -using `pip install -r requirements-dev.txt` in the root folder's `venv`)_. +This project uses `black` for code formatting, `Pylint` for linting, and +`Pyright` for static type checking. To run these commands, you need to have +installed the requirements in the repo's root declared in the +`requirements-dev.txt` file _(i.e. navigating to the root and using +`pip install -r requirements-dev.txt` in the root folder's `venv`)_. -Ensure you have activated your venv. You can then run the formatter and linter -by changing directory to the `controller-cli` folder and running: +Ensure you have activated your venv. You can then run the tools by running: + +The formatter: + +```sh + black pipelines/controller-cli +``` + +The type checker: ```sh - black . + pyright pipelines/controller-cli ``` You can run the linter with: diff --git a/requirements-dev.txt b/requirements-dev.txt index a9d2fc7bf..31ae2a1d2 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,2 +1,3 @@ black pylint +pyright From fde635595c4bce16cfd3377b49c8a89414df06e3 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Tue, 18 Nov 2025 20:43:20 +0300 Subject: [PATCH 2/2] Fixes Pyright warnings for Controller CLI --- pipelines/controller-cli/src/main.py | 29 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/pipelines/controller-cli/src/main.py b/pipelines/controller-cli/src/main.py index 2d37bb399..2b552eade 100644 --- a/pipelines/controller-cli/src/main.py +++ b/pipelines/controller-cli/src/main.py @@ -39,14 +39,17 @@ def process_response(response: str, args: argparse.Namespace): logger.info(f"Request url: {args.url}") logger.info("Response:") try: - logger.info(json.dumps(response, indent=4)) - except json.JSONDecodeError: + if isinstance(response, dict): + logger.info(json.dumps(response, indent=4)) + else: + logger.info(response) + except TypeError: logger.info(response) def _make_api_request( verb: str, url: str, params: Optional[Dict[str, Any]] = None -) -> Optional[Dict[str, Any]]: +) -> str: logger.debug(f"Making API request: {verb} {url}") logger.debug(f"Request parameters: {params}") try: @@ -103,11 +106,10 @@ def _make_api_request( logger.error(f"An error occurred during the request: {req_err}") except json.JSONDecodeError as json_err: logger.error(f"Failed to decode JSON response: {json_err}") - logger.error(response) - return None + return "API REQUEST FAILED" -def config(args: argparse.Namespace) -> str: +def config(args: argparse.Namespace) -> None: try: if args.config_name: response = _make_api_request( @@ -120,7 +122,7 @@ def config(args: argparse.Namespace) -> str: logger.error(f"Error processing: {e}") -def next_scheduled(args: argparse.Namespace) -> str: +def next_scheduled(args: argparse.Namespace) -> None: try: response = _make_api_request(HTTP_GET, f"{args.url}/next") process_response(response, args) @@ -128,7 +130,7 @@ def next_scheduled(args: argparse.Namespace) -> str: logger.error(f"Error processing: {e}") -def status(args: argparse.Namespace) -> str: +def status(args: argparse.Namespace) -> None: try: response = _make_api_request(HTTP_GET, f"{args.url}/status") process_response(response, args) @@ -136,7 +138,7 @@ def status(args: argparse.Namespace) -> str: logger.error(f"Error processing: {e}") -def run(args: argparse.Namespace) -> str: +def run(args: argparse.Namespace) -> None: logger.info("=" * 50) logger.info("Executing 'run' command - starting pipeline run") logger.info(f"Run mode: {args.mode}") @@ -164,10 +166,9 @@ def run(args: argparse.Namespace) -> str: logger.error(f"Error in run command: {e}") logger.error(f"Failed to execute pipeline run with mode: {args.mode}") logger.error("=" * 50) - logger.error(f"Error processing: {e}") -def tables(args: argparse.Namespace) -> str: +def tables(args: argparse.Namespace) -> None: try: response = _make_api_request(HTTP_POST, f"{args.url}/tables") process_response(response, args) @@ -185,11 +186,11 @@ def download_file(url: str, filename: str) -> str: return f"Error downloading file: {e}" -def logs(args: argparse.Namespace) -> str: +def logs(args: argparse.Namespace) -> None: try: if args.download: filename = args.filename if args.filename else "error.log" - response = download_file(f"{args.url}/download-error-log", filename) + response: str = download_file(f"{args.url}/download-error-log", filename) process_response(response, args) else: logger.info( @@ -211,7 +212,7 @@ def delete_snapshot(args: argparse.Namespace) -> str: return f"Error deleting snapshot: {e}" -def dwh(args: argparse.Namespace) -> str: +def dwh(args: argparse.Namespace) -> None: try: if hasattr(args, "snapshot_id"): response = delete_snapshot(args)