From fbb164e003129b1bf5dd4c45f794e9d692c64418 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Fri, 18 Jul 2025 16:38:58 +0000 Subject: [PATCH 01/13] feat: read content of generate request --- .generator/cli.py | 31 +++++++++++++++++++++++++ .generator/requirements-test.in | 16 +++++++++++++ .generator/test_cli.py | 39 +++++++++++++++++++++++++++++++- .librarian/generate-request.json | 10 ++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 .generator/requirements-test.in create mode 100644 .librarian/generate-request.json diff --git a/.generator/cli.py b/.generator/cli.py index 09dfbe15746f..62c4e6511677 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -14,6 +14,26 @@ import argparse import sys +import json +import os +import logging + +logger = logging.getLogger() + +LIBRARIAN = "/librarian" + +# Helper function that reads a json file path and returns the loaded json content. +def _read_json_file(path): + if not os.path.exists(path): + raise FileNotFoundError(f"Request file not found at '{path}'") + try: + with open(path, 'r') as f: + return json.load(f) + except json.JSONDecodeError: + raise ValueError(f"Invalid JSON in request file '{path}'") + except IOError as e: + raise IOError(f"Invalid JSON in request file '{path}': {e}") + def handle_configure(dry_run=False): # TODO(https://github.com/googleapis/librarian/issues/466): Implement configure command. @@ -21,6 +41,17 @@ def handle_configure(dry_run=False): def handle_generate(dry_run=False): # TODO(https://github.com/googleapis/librarian/issues/448): Implement generate command. + + # Read a generate-request.json file + request_path = f"{LIBRARIAN}/generate-request.json" + try: + request_data = _read_json_file(request_path) + except Exception as e: + logger.error(e) + sys.exit(1) + + # Print the data: + print(json.dumps(request_data, indent=2)) print("'generate' command executed.") def handle_build(dry_run=False): diff --git a/.generator/requirements-test.in b/.generator/requirements-test.in new file mode 100644 index 000000000000..c09827ec2eeb --- /dev/null +++ b/.generator/requirements-test.in @@ -0,0 +1,16 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +pytest +pytest-mock diff --git a/.generator/test_cli.py b/.generator/test_cli.py index 1f77c88ffa39..8a7fe9ec83c1 100644 --- a/.generator/test_cli.py +++ b/.generator/test_cli.py @@ -12,7 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -from cli import handle_generate, handle_build, handle_configure +import pytest +import json + +from cli import _read_json_file, handle_generate, handle_build, handle_configure def test_handle_configure_dry_run(): @@ -26,3 +29,37 @@ def test_handle_generate_dry_run(): def test_handle_build_dry_run(): # This is a simple test to ensure that the dry run command succeeds. handle_build(dry_run=True) + +def test_read_valid_json(mocker): + """Tests reading a valid JSON file.""" + mock_content = '{"key": "value"}' + mocker.patch("os.path.exists", return_value=True) + mocker.patch("builtins.open", mocker.mock_open(read_data=mock_content)) + result = _read_json_file("fake/path.json") + assert result == {"key": "value"} + +def test_file_not_found(mocker): + """Tests behavior when the file does not exist.""" + mocker.patch("os.path.exists", return_value=False) + + with pytest.raises(FileNotFoundError): + _read_json_file("non/existent/path.json") + +def test_invalid_json(mocker): + """Tests reading a file with malformed JSON.""" + mock_content = '{"key": "value",}' + mocker.patch("os.path.exists", return_value=True) + mocker.patch("builtins.open", mocker.mock_open(read_data=mock_content)) + + with pytest.raises(ValueError, match="Invalid JSON"): + _read_json_file("fake/path.json") + +def test_io_error_on_read(mocker): + """Tests for a generic IOError.""" + mocker.patch("os.path.exists", return_value=True) + mocked_open = mocker.mock_open() + mocked_open.side_effect = IOError("Permission denied") + mocker.patch("builtins.open", mocked_open) + + with pytest.raises(IOError, match="Permission denied"): + _read_json_file("fake/path.json") diff --git a/.librarian/generate-request.json b/.librarian/generate-request.json new file mode 100644 index 000000000000..685420f7a312 --- /dev/null +++ b/.librarian/generate-request.json @@ -0,0 +1,10 @@ +{ + "id": "google-cloud-language", + "apis": [ + { + "path": "google/cloud/language/v1", + "service_config": "language.yaml" + } + ] + } + \ No newline at end of file From 0b2264f0c3762d73d57b7f1ea0769f8149742501 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Jul 2025 19:46:05 +0000 Subject: [PATCH 02/13] test commit --- .generator/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/.generator/cli.py b/.generator/cli.py index 62c4e6511677..cb97fddd6d8a 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -23,6 +23,7 @@ LIBRARIAN = "/librarian" # Helper function that reads a json file path and returns the loaded json content. +# Remove me. def _read_json_file(path): if not os.path.exists(path): raise FileNotFoundError(f"Request file not found at '{path}'") From 456b3f738f42bcf3fb4fc5d66078cf28240ec244 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Jul 2025 20:11:57 +0000 Subject: [PATCH 03/13] add cloud build config --- cloudbuild.yaml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 cloudbuild.yaml diff --git a/cloudbuild.yaml b/cloudbuild.yaml new file mode 100644 index 000000000000..116a34773305 --- /dev/null +++ b/cloudbuild.yaml @@ -0,0 +1,28 @@ +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +steps: + # This step builds the Docker image. + - name: 'gcr.io/cloud-builders/docker' + args: + - 'build' + - '-t' + - 'gcr.io/$PROJECT_ID/python-librarian-generator:latest' + - '-f' + - '.generator/Dockerfile' + - '.' + +# This section solves the logging error by automatically creating a bucket. +options: + default_logs_bucket_behavior: REGIONAL_USER_OWNED_BUCKET From 5d48e802845e95996170eab3a18e266ddfabdbdd Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Jul 2025 20:54:20 +0000 Subject: [PATCH 04/13] address build failure --- .generator/Dockerfile | 3 +-- cloudbuild.yaml | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.generator/Dockerfile b/.generator/Dockerfile index 2f64101ea8cc..ec620b338b40 100644 --- a/.generator/Dockerfile +++ b/.generator/Dockerfile @@ -64,8 +64,7 @@ WORKDIR /app # TODO(https://github.com/googleapis/librarian/issues/906): Clone googleapis and run bazelisk build. # Copy the CLI script into the container and set ownership. -# No other files or dependencies are needed for this simple script. -COPY cli.py . +COPY .generator/cli.py . # Set the entrypoint for the container to run the script. ENTRYPOINT ["python3.13", "./cli.py"] diff --git a/cloudbuild.yaml b/cloudbuild.yaml index 116a34773305..1ff7242d654e 100644 --- a/cloudbuild.yaml +++ b/cloudbuild.yaml @@ -1,4 +1,4 @@ -# Copyright 2022 Google LLC +# Copyright 2025 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From 7dfd7bf1960c1dad3ae93a1289fe9a7282898d48 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 04:19:24 +0000 Subject: [PATCH 05/13] address PR feedback --- .generator/cli.py | 51 ++++++++++++++++++++++-------------------- .generator/test_cli.py | 12 +++++++--- 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/.generator/cli.py b/.generator/cli.py index cb97fddd6d8a..73c899bba0a8 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -13,48 +13,45 @@ # limitations under the License. import argparse -import sys import json -import os import logging +import os +import sys logger = logging.getLogger() LIBRARIAN = "/librarian" + # Helper function that reads a json file path and returns the loaded json content. -# Remove me. def _read_json_file(path): - if not os.path.exists(path): - raise FileNotFoundError(f"Request file not found at '{path}'") - try: - with open(path, 'r') as f: - return json.load(f) - except json.JSONDecodeError: - raise ValueError(f"Invalid JSON in request file '{path}'") - except IOError as e: - raise IOError(f"Invalid JSON in request file '{path}': {e}") + with open(path, "r") as f: + return json.load(f) def handle_configure(dry_run=False): # TODO(https://github.com/googleapis/librarian/issues/466): Implement configure command. print("'configure' command executed.") + def handle_generate(dry_run=False): - # TODO(https://github.com/googleapis/librarian/issues/448): Implement generate command. # Read a generate-request.json file - request_path = f"{LIBRARIAN}/generate-request.json" - try: - request_data = _read_json_file(request_path) - except Exception as e: - logger.error(e) - sys.exit(1) + if not dry_run: + request_path = f"{LIBRARIAN}/generate-request.json" + try: + request_data = _read_json_file(request_path) + except Exception as e: + logger.error(e) + sys.exit(1) + + # Print the data: + print(json.dumps(request_data, indent=2)) - # Print the data: - print(json.dumps(request_data, indent=2)) + # TODO(https://github.com/googleapis/librarian/issues/448): Implement generate command. print("'generate' command executed.") + def handle_build(dry_run=False): # TODO(https://github.com/googleapis/librarian/issues/450): Implement build command. print("'build' command executed.") @@ -62,15 +59,21 @@ def handle_build(dry_run=False): if __name__ == "__main__": parser = argparse.ArgumentParser(description="A simple CLI tool.") - subparsers = parser.add_subparsers(dest="command", required=True, help="Available commands") + subparsers = parser.add_subparsers( + dest="command", required=True, help="Available commands" + ) # Define commands for command_name, help_text in [ ("configure", "Onboard a new library or an api path to Librarian workflow."), ("generate", "generate a python client for an API."), - ("build", "Run unit tests via nox for the generated library.") + ("build", "Run unit tests via nox for the generated library."), ]: - handler_map = {"configure": handle_configure, "generate": handle_generate, "build": handle_build} + handler_map = { + "configure": handle_configure, + "generate": handle_generate, + "build": handle_build, + } parser_cmd = subparsers.add_parser(command_name, help=help_text) parser_cmd.set_defaults(func=handler_map[command_name]) diff --git a/.generator/test_cli.py b/.generator/test_cli.py index 8a7fe9ec83c1..5b64daa002d5 100644 --- a/.generator/test_cli.py +++ b/.generator/test_cli.py @@ -22,14 +22,17 @@ def test_handle_configure_dry_run(): # This is a simple test to ensure that the dry run command succeeds. handle_configure(dry_run=True) + def test_handle_generate_dry_run(): # This is a simple test to ensure that the dry run command succeeds. handle_generate(dry_run=True) + def test_handle_build_dry_run(): # This is a simple test to ensure that the dry run command succeeds. handle_build(dry_run=True) + def test_read_valid_json(mocker): """Tests reading a valid JSON file.""" mock_content = '{"key": "value"}' @@ -38,6 +41,7 @@ def test_read_valid_json(mocker): result = _read_json_file("fake/path.json") assert result == {"key": "value"} + def test_file_not_found(mocker): """Tests behavior when the file does not exist.""" mocker.patch("os.path.exists", return_value=False) @@ -45,21 +49,23 @@ def test_file_not_found(mocker): with pytest.raises(FileNotFoundError): _read_json_file("non/existent/path.json") + def test_invalid_json(mocker): """Tests reading a file with malformed JSON.""" mock_content = '{"key": "value",}' mocker.patch("os.path.exists", return_value=True) mocker.patch("builtins.open", mocker.mock_open(read_data=mock_content)) - with pytest.raises(ValueError, match="Invalid JSON"): + with pytest.raises(json.JSONDecodeError): _read_json_file("fake/path.json") + def test_io_error_on_read(mocker): """Tests for a generic IOError.""" mocker.patch("os.path.exists", return_value=True) mocked_open = mocker.mock_open() - mocked_open.side_effect = IOError("Permission denied") + mocked_open.side_effect = IOError("permission denied") mocker.patch("builtins.open", mocked_open) - with pytest.raises(IOError, match="Permission denied"): + with pytest.raises(IOError): _read_json_file("fake/path.json") From ac7f6f257d6ee471aa0ca886f3d5a34e065e34d7 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 05:40:23 +0000 Subject: [PATCH 06/13] address PR feedback --- .generator/cli.py | 15 +++++++++++--- .../generate-request.json | 1 - .generator/test_cli.py | 20 ++++++++++++++++++- 3 files changed, 31 insertions(+), 5 deletions(-) rename {.librarian => .generator}/generate-request.json (98%) diff --git a/.generator/cli.py b/.generator/cli.py index 73c899bba0a8..b94bc8426c38 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -20,7 +20,17 @@ logger = logging.getLogger() -LIBRARIAN = "/librarian" +LIBRARIAN_DIR = "/librarian" +GENERATOR_DIR = "/.generator" +GENERATE_REQUEST_FILE = "generate-request.json" + + +def _get_base_dir(): + """Returns the correct base directory based on the environment.""" + environment = os.getenv("PY_GENERATOR_ENV", "production").lower() + if environment == "test": + return GENERATOR_DIR + return LIBRARIAN_DIR # Helper function that reads a json file path and returns the loaded json content. @@ -38,9 +48,8 @@ def handle_generate(dry_run=False): # Read a generate-request.json file if not dry_run: - request_path = f"{LIBRARIAN}/generate-request.json" try: - request_data = _read_json_file(request_path) + request_data = _read_json_file(f"{_get_base_dir()}/{GENERATE_REQUEST_FILE}") except Exception as e: logger.error(e) sys.exit(1) diff --git a/.librarian/generate-request.json b/.generator/generate-request.json similarity index 98% rename from .librarian/generate-request.json rename to .generator/generate-request.json index 685420f7a312..f673a05a509a 100644 --- a/.librarian/generate-request.json +++ b/.generator/generate-request.json @@ -7,4 +7,3 @@ } ] } - \ No newline at end of file diff --git a/.generator/test_cli.py b/.generator/test_cli.py index 5b64daa002d5..93c45eadabb4 100644 --- a/.generator/test_cli.py +++ b/.generator/test_cli.py @@ -15,7 +15,25 @@ import pytest import json -from cli import _read_json_file, handle_generate, handle_build, handle_configure +from unittest.mock import mock_open + +from cli import ( + _read_json_file, + handle_generate, + handle_build, + handle_configure, + GENERATOR_DIR, + GENERATE_REQUEST_FILE, +) + + +# # The fixture is defined directly in the test file +@pytest.fixture(autouse=True) +def set_test_environment(monkeypatch): + """ + Sets the environment variable only for tests in this file. + """ + monkeypatch.setenv("PY_GENERATOR_ENV", "test") def test_handle_configure_dry_run(): From cca01f6d35bb1e21ca7636c72bdb83ad5aa11a18 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 06:23:58 +0000 Subject: [PATCH 07/13] address PR feedback --- .generator/cli.py | 9 ++++++++- .github/workflows/generator.yml | 5 +++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.generator/cli.py b/.generator/cli.py index b94bc8426c38..3e303734a7d4 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -72,6 +72,13 @@ def handle_build(dry_run=False): dest="command", required=True, help="Available commands" ) + # This flag is needed for testing. + parser.add_argument( + "--dry-run", + action="store_true", + help="Perform a dry run for testing purposes." + ) + # Define commands for command_name, help_text in [ ("configure", "Onboard a new library or an api path to Librarian workflow."), @@ -91,4 +98,4 @@ def handle_build(dry_run=False): sys.exit(1) args = parser.parse_args() - args.func(args) + args.func(dry_run=args.dry_run) diff --git a/.github/workflows/generator.yml b/.github/workflows/generator.yml index 447c24df88e0..a9d2c3d9eec5 100644 --- a/.github/workflows/generator.yml +++ b/.github/workflows/generator.yml @@ -22,9 +22,10 @@ jobs: uses: actions/setup-python@v5 with: python-version: "3.10" - - name: Install pytest + - name: Install dependencies run: | - python -m pip install pytest + python -m pip install --upgrade pip + pip install -r .generator/requirements-test.in - name: Run generator_cli tests run: | pytest .generator/test_cli.py From 1130c70ee1b283a55e01dabea4a7915acde56596 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 15:33:54 +0000 Subject: [PATCH 08/13] address PR feedback --- .generator/Dockerfile | 7 +++---- .generator/cli.py | 13 +++++++------ .github/workflows/generator.yml | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.generator/Dockerfile b/.generator/Dockerfile index ec620b338b40..2785e9d5c2ef 100644 --- a/.generator/Dockerfile +++ b/.generator/Dockerfile @@ -34,11 +34,10 @@ RUN apt-get update && \ rm -rf /var/lib/apt/lists/* # Set up environment variables for tool versions to make updates easier. -# TODO(): Downgrade to Python `3.11` if `3.13.5` is incompatible with googleapis. -ENV PYTHON_VERSION=3.13.5 +ENV PYTHON_VERSION=3.11.5 # Create a symbolic link for `python3` to point to our specific version. -ENV PATH /usr/local/bin/python3.13:$PATH +ENV PATH /usr/local/bin/python3.11:$PATH # Install Python from source RUN wget https://www.python.org/ftp/python/${PYTHON_VERSION}/Python-${PYTHON_VERSION}.tgz && \ @@ -67,4 +66,4 @@ WORKDIR /app COPY .generator/cli.py . # Set the entrypoint for the container to run the script. -ENTRYPOINT ["python3.13", "./cli.py"] +ENTRYPOINT ["python3.11", "./cli.py"] diff --git a/.generator/cli.py b/.generator/cli.py index 3e303734a7d4..4d404883ff8c 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -21,7 +21,7 @@ logger = logging.getLogger() LIBRARIAN_DIR = "/librarian" -GENERATOR_DIR = "/.generator" +GENERATOR_DIR = ".generator" GENERATE_REQUEST_FILE = "generate-request.json" @@ -80,16 +80,17 @@ def handle_build(dry_run=False): ) # Define commands + handler_map = { + "configure": handle_configure, + "generate": handle_generate, + "build": handle_build, + } + for command_name, help_text in [ ("configure", "Onboard a new library or an api path to Librarian workflow."), ("generate", "generate a python client for an API."), ("build", "Run unit tests via nox for the generated library."), ]: - handler_map = { - "configure": handle_configure, - "generate": handle_generate, - "build": handle_build, - } parser_cmd = subparsers.add_parser(command_name, help=help_text) parser_cmd.set_defaults(func=handler_map[command_name]) diff --git a/.github/workflows/generator.yml b/.github/workflows/generator.yml index a9d2c3d9eec5..77d5f05daffa 100644 --- a/.github/workflows/generator.yml +++ b/.github/workflows/generator.yml @@ -21,7 +21,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v5 with: - python-version: "3.10" + python-version: "3.11" - name: Install dependencies run: | python -m pip install --upgrade pip From 98fb3c969cae8f45f57c1d969f09d0a32b2709e3 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 15:40:20 +0000 Subject: [PATCH 09/13] clean up unit tests --- .generator/test_cli.py | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/.generator/test_cli.py b/.generator/test_cli.py index 93c45eadabb4..4255cb9b24ac 100644 --- a/.generator/test_cli.py +++ b/.generator/test_cli.py @@ -54,7 +54,6 @@ def test_handle_build_dry_run(): def test_read_valid_json(mocker): """Tests reading a valid JSON file.""" mock_content = '{"key": "value"}' - mocker.patch("os.path.exists", return_value=True) mocker.patch("builtins.open", mocker.mock_open(read_data=mock_content)) result = _read_json_file("fake/path.json") assert result == {"key": "value"} @@ -62,7 +61,7 @@ def test_read_valid_json(mocker): def test_file_not_found(mocker): """Tests behavior when the file does not exist.""" - mocker.patch("os.path.exists", return_value=False) + mocker.patch("builtins.open", side_effect=FileNotFoundError("No such file")) with pytest.raises(FileNotFoundError): _read_json_file("non/existent/path.json") @@ -71,19 +70,7 @@ def test_file_not_found(mocker): def test_invalid_json(mocker): """Tests reading a file with malformed JSON.""" mock_content = '{"key": "value",}' - mocker.patch("os.path.exists", return_value=True) mocker.patch("builtins.open", mocker.mock_open(read_data=mock_content)) with pytest.raises(json.JSONDecodeError): _read_json_file("fake/path.json") - - -def test_io_error_on_read(mocker): - """Tests for a generic IOError.""" - mocker.patch("os.path.exists", return_value=True) - mocked_open = mocker.mock_open() - mocked_open.side_effect = IOError("permission denied") - mocker.patch("builtins.open", mocked_open) - - with pytest.raises(IOError): - _read_json_file("fake/path.json") From 647d70e6f73074ed7f6ac49040b2bd08078b9288 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 16:06:33 +0000 Subject: [PATCH 10/13] simplify testing logic --- .generator/cli.py | 19 ++++------------ .generator/generate-request.json | 9 -------- .generator/test_cli.py | 39 +++++++++++++++++++++++++------- 3 files changed, 35 insertions(+), 32 deletions(-) delete mode 100644 .generator/generate-request.json diff --git a/.generator/cli.py b/.generator/cli.py index 4d404883ff8c..c92bc353e558 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -20,19 +20,10 @@ logger = logging.getLogger() -LIBRARIAN_DIR = "/librarian" -GENERATOR_DIR = ".generator" +LIBRARIAN_DIR = "librarian" GENERATE_REQUEST_FILE = "generate-request.json" -def _get_base_dir(): - """Returns the correct base directory based on the environment.""" - environment = os.getenv("PY_GENERATOR_ENV", "production").lower() - if environment == "test": - return GENERATOR_DIR - return LIBRARIAN_DIR - - # Helper function that reads a json file path and returns the loaded json content. def _read_json_file(path): with open(path, "r") as f: @@ -49,7 +40,7 @@ def handle_generate(dry_run=False): # Read a generate-request.json file if not dry_run: try: - request_data = _read_json_file(f"{_get_base_dir()}/{GENERATE_REQUEST_FILE}") + request_data = _read_json_file(f"{LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}") except Exception as e: logger.error(e) sys.exit(1) @@ -74,9 +65,7 @@ def handle_build(dry_run=False): # This flag is needed for testing. parser.add_argument( - "--dry-run", - action="store_true", - help="Perform a dry run for testing purposes." + "--dry-run", action="store_true", help="Perform a dry run for testing purposes." ) # Define commands @@ -85,7 +74,7 @@ def handle_build(dry_run=False): "generate": handle_generate, "build": handle_build, } - + for command_name, help_text in [ ("configure", "Onboard a new library or an api path to Librarian workflow."), ("generate", "generate a python client for an API."), diff --git a/.generator/generate-request.json b/.generator/generate-request.json deleted file mode 100644 index f673a05a509a..000000000000 --- a/.generator/generate-request.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "id": "google-cloud-language", - "apis": [ - { - "path": "google/cloud/language/v1", - "service_config": "language.yaml" - } - ] - } diff --git a/.generator/test_cli.py b/.generator/test_cli.py index 4255cb9b24ac..488c2c4f6aec 100644 --- a/.generator/test_cli.py +++ b/.generator/test_cli.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os import pytest import json @@ -22,18 +23,29 @@ handle_generate, handle_build, handle_configure, - GENERATOR_DIR, + LIBRARIAN_DIR, GENERATE_REQUEST_FILE, ) -# # The fixture is defined directly in the test file -@pytest.fixture(autouse=True) -def set_test_environment(monkeypatch): - """ - Sets the environment variable only for tests in this file. - """ - monkeypatch.setenv("PY_GENERATOR_ENV", "test") +@pytest.fixture +def mock_generate_request_file(tmp_path, monkeypatch): + """Creates the mock request file at the correct path inside a temp dir.""" + # Create the path as expected by the script: .librarian/generate-request.json + request_path = f"{LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}" + request_dir = tmp_path / os.path.dirname(request_path) + request_dir.mkdir() + request_file = request_dir / os.path.basename(request_path) + + request_content = { + "id": "google-cloud-language", + "apis": [{"path": "google/cloud/language/v1"}], + } + request_file.write_text(json.dumps(request_content)) + + # Change the current working directory to the temp path for the test. + monkeypatch.chdir(tmp_path) + return request_file def test_handle_configure_dry_run(): @@ -46,6 +58,17 @@ def test_handle_generate_dry_run(): handle_generate(dry_run=True) +def test_handle_generate_success(capsys, mock_generate_request_file): + """ + Tests the successful execution path of handle_generate. + """ + handle_generate(dry_run=False) + + captured = capsys.readouterr() + assert "google-cloud-language" in captured.out + assert "'generate' command executed." in captured.out + + def test_handle_build_dry_run(): # This is a simple test to ensure that the dry run command succeeds. handle_build(dry_run=True) From 75a09315bcc44c9b62861c8bf683f0d37ca6cbcd Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 16:15:54 +0000 Subject: [PATCH 11/13] clarify storage bucket comment --- cloudbuild.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudbuild.yaml b/cloudbuild.yaml index 1ff7242d654e..2a7dd688e1af 100644 --- a/cloudbuild.yaml +++ b/cloudbuild.yaml @@ -23,6 +23,6 @@ steps: - '.generator/Dockerfile' - '.' -# This section solves the logging error by automatically creating a bucket. +# This section automatically create a storage bucket for storing docker build logs. options: default_logs_bucket_behavior: REGIONAL_USER_OWNED_BUCKET From f0e8bda03a5e17f55b8960d647b5559406e8670b Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 16:51:38 +0000 Subject: [PATCH 12/13] change print statements to logs --- .generator/cli.py | 11 +++++------ .generator/test_cli.py | 10 ++++++---- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.generator/cli.py b/.generator/cli.py index c92bc353e558..662546138949 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -32,7 +32,7 @@ def _read_json_file(path): def handle_configure(dry_run=False): # TODO(https://github.com/googleapis/librarian/issues/466): Implement configure command. - print("'configure' command executed.") + logger.info("'configure' command executed.") def handle_generate(dry_run=False): @@ -42,19 +42,18 @@ def handle_generate(dry_run=False): try: request_data = _read_json_file(f"{LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}") except Exception as e: - logger.error(e) + logger.error(f"failed to read {LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}: {e}") sys.exit(1) - # Print the data: - print(json.dumps(request_data, indent=2)) + logger.info(json.dumps(request_data, indent=2)) # TODO(https://github.com/googleapis/librarian/issues/448): Implement generate command. - print("'generate' command executed.") + logger.info("'generate' command executed.") def handle_build(dry_run=False): # TODO(https://github.com/googleapis/librarian/issues/450): Implement build command. - print("'build' command executed.") + logger.info("'build' command executed.") if __name__ == "__main__": diff --git a/.generator/test_cli.py b/.generator/test_cli.py index 488c2c4f6aec..5386edd64a52 100644 --- a/.generator/test_cli.py +++ b/.generator/test_cli.py @@ -15,6 +15,7 @@ import os import pytest import json +import logging from unittest.mock import mock_open @@ -58,15 +59,16 @@ def test_handle_generate_dry_run(): handle_generate(dry_run=True) -def test_handle_generate_success(capsys, mock_generate_request_file): +def test_handle_generate_success(caplog, mock_generate_request_file): """ Tests the successful execution path of handle_generate. """ + caplog.set_level(logging.INFO) + handle_generate(dry_run=False) - captured = capsys.readouterr() - assert "google-cloud-language" in captured.out - assert "'generate' command executed." in captured.out + assert "google-cloud-language" in caplog.text + assert "'generate' command executed." in caplog.text def test_handle_build_dry_run(): From d785d920c71342fb972662cf1d9fb4573702124b Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Jul 2025 17:55:36 +0000 Subject: [PATCH 13/13] address PR feedback --- .generator/cli.py | 56 +++++++++++++++++++++++++++--------------- .generator/test_cli.py | 35 ++++++++++++++++++-------- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/.generator/cli.py b/.generator/cli.py index 662546138949..fe87ce1a6b09 100644 --- a/.generator/cli.py +++ b/.generator/cli.py @@ -24,35 +24,56 @@ GENERATE_REQUEST_FILE = "generate-request.json" -# Helper function that reads a json file path and returns the loaded json content. def _read_json_file(path): + """Helper function that reads a json file path and returns the loaded json content. + + Args: + path (str): The file path to read. + + Returns: + dict: The parsed JSON content. + + Raises: + FileNotFoundError: If the file is not found at the specified path. + json.JSONDecodeError: If the file does not contain valid JSON. + IOError: If there is an issue reading the file. + """ with open(path, "r") as f: return json.load(f) -def handle_configure(dry_run=False): - # TODO(https://github.com/googleapis/librarian/issues/466): Implement configure command. +def handle_configure(): + # TODO(https://github.com/googleapis/librarian/issues/466): Implement configure command and update docstring. logger.info("'configure' command executed.") -def handle_generate(dry_run=False): +def handle_generate(): + """The main coordinator for the code generation process. + + This function orchestrates the generation of a client library by reading a + `librarian/generate-request.json` file, determining the necessary Bazel rule for each API, and + (in future steps) executing the build. + + Raises: + ValueError: If the `generate-request.json` file is not found or read. + """ # Read a generate-request.json file - if not dry_run: - try: - request_data = _read_json_file(f"{LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}") - except Exception as e: - logger.error(f"failed to read {LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}: {e}") - sys.exit(1) + try: + request_data = _read_json_file(f"{LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}") + except Exception as e: + raise ValueError( + f"failed to read {LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}" + ) from e - logger.info(json.dumps(request_data, indent=2)) + logger.info(json.dumps(request_data, indent=2)) - # TODO(https://github.com/googleapis/librarian/issues/448): Implement generate command. + # TODO(https://github.com/googleapis/librarian/issues/448): Implement generate command and update docstring. logger.info("'generate' command executed.") -def handle_build(dry_run=False): - # TODO(https://github.com/googleapis/librarian/issues/450): Implement build command. +def handle_build(): + # TODO(https://github.com/googleapis/librarian/issues/450): Implement build command and update docstring. logger.info("'build' command executed.") @@ -62,11 +83,6 @@ def handle_build(dry_run=False): dest="command", required=True, help="Available commands" ) - # This flag is needed for testing. - parser.add_argument( - "--dry-run", action="store_true", help="Perform a dry run for testing purposes." - ) - # Define commands handler_map = { "configure": handle_configure, @@ -87,4 +103,4 @@ def handle_build(dry_run=False): sys.exit(1) args = parser.parse_args() - args.func(dry_run=args.dry_run) + args.func() diff --git a/.generator/test_cli.py b/.generator/test_cli.py index 5386edd64a52..cbf73bdfaea3 100644 --- a/.generator/test_cli.py +++ b/.generator/test_cli.py @@ -49,14 +49,15 @@ def mock_generate_request_file(tmp_path, monkeypatch): return request_file -def test_handle_configure_dry_run(): - # This is a simple test to ensure that the dry run command succeeds. - handle_configure(dry_run=True) +def test_handle_configure_success(caplog, mock_generate_request_file): + """ + Tests the successful execution path of handle_configure. + """ + caplog.set_level(logging.INFO) + handle_configure() -def test_handle_generate_dry_run(): - # This is a simple test to ensure that the dry run command succeeds. - handle_generate(dry_run=True) + assert "'configure' command executed." in caplog.text def test_handle_generate_success(caplog, mock_generate_request_file): @@ -65,15 +66,29 @@ def test_handle_generate_success(caplog, mock_generate_request_file): """ caplog.set_level(logging.INFO) - handle_generate(dry_run=False) + handle_generate() assert "google-cloud-language" in caplog.text assert "'generate' command executed." in caplog.text -def test_handle_build_dry_run(): - # This is a simple test to ensure that the dry run command succeeds. - handle_build(dry_run=True) +def test_handle_generate_fail(caplog): + """ + Tests the failed to read `librarian/generate-request.json` file in handle_generates. + """ + with pytest.raises(ValueError): + handle_generate() + + +def test_handle_build_success(caplog, mock_generate_request_file): + """ + Tests the successful execution path of handle_build. + """ + caplog.set_level(logging.INFO) + + handle_build() + + assert "'build' command executed." in caplog.text def test_read_valid_json(mocker):