From 8866d637acc0fc492fdb1ebe878eb22d60eb99da Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Wed, 2 Aug 2023 10:54:40 -0700 Subject: [PATCH 01/11] support file-based config --- action/deploy_recipe.py | 11 ++++++++++- action/test_main.py | 17 +++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/action/deploy_recipe.py b/action/deploy_recipe.py index d4b3526..d5b3d68 100644 --- a/action/deploy_recipe.py +++ b/action/deploy_recipe.py @@ -40,9 +40,18 @@ def main(): run_attempt = os.environ["GITHUB_RUN_ATTEMPT"] # user input - config = json.loads(os.environ["INPUT_PANGEO_FORGE_RUNNER_CONFIG"]) + config_string = os.environ["INPUT_PANGEO_FORGE_RUNNER_CONFIG"] select_recipe_by_label = os.environ["INPUT_SELECT_RECIPE_BY_LABEL"] + # parse config + if os.path.exists(config_string): + # we allow local paths pointing to json files + with open(config_string) as f: + config = json.load(f) + else: + # or json strings passed inline in the workflow yaml + config = json.loads(config_string) + # log variables to stdout print(f"{conda_env = }") print(f"{head_ref = }") diff --git a/action/test_main.py b/action/test_main.py index 0b16e30..7474d86 100644 --- a/action/test_main.py +++ b/action/test_main.py @@ -1,3 +1,4 @@ +import json import os from unittest.mock import MagicMock, patch @@ -16,8 +17,20 @@ def select_recipe_by_label(request): return request.param +@pytest.fixture(params=["string", "file"]) +def pangeo_forge_runner_config(request, tmp_path_factory): + config_string = '{"a": "b"}' + if request.param == "file": + fn = tmp_path_factory.mktemp("config") / "pangeo-forge-runner-config.json" + with open(fn.name, mode="w") as f: + json.dump(config_string, f) + return fn.name + else: + return config_string + + @pytest.fixture -def env(select_recipe_by_label, head_ref): +def env(select_recipe_by_label, head_ref, pangeo_forge_runner_config): return { "CONDA_ENV": "notebook", "GITHUB_REPOSITORY": "my/repo", @@ -30,7 +43,7 @@ def env(select_recipe_by_label, head_ref): "GITHUB_RUN_ID": "0987654321", "GITHUB_RUN_ATTEMPT": "1", # TODO: parametrize runner config with `BaseCommand.feedstock-subdir` - "INPUT_PANGEO_FORGE_RUNNER_CONFIG": '{"a": "b"}', + "INPUT_PANGEO_FORGE_RUNNER_CONFIG": pangeo_forge_runner_config, "INPUT_SELECT_RECIPE_BY_LABEL": select_recipe_by_label, } From 71930ea9cab16d81d31896fe4f63a66b320f05be Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Wed, 2 Aug 2023 11:19:16 -0700 Subject: [PATCH 02/11] add error handling --- action/deploy_recipe.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/action/deploy_recipe.py b/action/deploy_recipe.py index d5b3d68..ba76636 100644 --- a/action/deploy_recipe.py +++ b/action/deploy_recipe.py @@ -44,13 +44,23 @@ def main(): select_recipe_by_label = os.environ["INPUT_SELECT_RECIPE_BY_LABEL"] # parse config + print(f"pangeo-forge-runner-config provided as {config_string}") if os.path.exists(config_string): # we allow local paths pointing to json files + print(f"Loading json from file '{config_string}'...") with open(config_string) as f: config = json.load(f) else: # or json strings passed inline in the workflow yaml - config = json.loads(config_string) + print(f"{config_string} does not exist as a file. Loading json from string...") + try: + config = json.loads(config_string) + except json.JSONDecodeError as e: + raise ValueError( + f"{config_string} failed to parse to JSON. If you meant to pass a JSON string, " + "please confirm that it is correctly formatted. If you meant to pass a filename, " + f"please confirm this file exists at the given path, relative to {os.getcwd()}." + ) from e # log variables to stdout print(f"{conda_env = }") From f5e9ba9f0932100ea9c5b602fd7004c16913a927 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Wed, 2 Aug 2023 11:24:14 -0700 Subject: [PATCH 03/11] fix tempdir fixture --- action/test_main.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/action/test_main.py b/action/test_main.py index 7474d86..9bc490b 100644 --- a/action/test_main.py +++ b/action/test_main.py @@ -22,9 +22,10 @@ def pangeo_forge_runner_config(request, tmp_path_factory): config_string = '{"a": "b"}' if request.param == "file": fn = tmp_path_factory.mktemp("config") / "pangeo-forge-runner-config.json" - with open(fn.name, mode="w") as f: + resolved_path = fn.resolve() + with open(resolved_path, mode="w") as f: json.dump(config_string, f) - return fn.name + return str(resolved_path) else: return config_string From e7ec8c7521e636c37fd5fc7f2b125941066cbc5d Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Wed, 2 Aug 2023 11:31:06 -0700 Subject: [PATCH 04/11] tweak error message --- action/deploy_recipe.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/action/deploy_recipe.py b/action/deploy_recipe.py index ba76636..ccd0812 100644 --- a/action/deploy_recipe.py +++ b/action/deploy_recipe.py @@ -59,7 +59,9 @@ def main(): raise ValueError( f"{config_string} failed to parse to JSON. If you meant to pass a JSON string, " "please confirm that it is correctly formatted. If you meant to pass a filename, " - f"please confirm this file exists at the given path, relative to {os.getcwd()}." + "please confirm this file exists at the given path. Note, this action is running " + f"where the workflow YAML file lives, which is: '{os.getcwd()}'. If you provide " + "a relative path, it must be relative to this location." ) from e # log variables to stdout From c13b7f056229f88ba47bbcbf13960038e13b1fa3 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Wed, 2 Aug 2023 12:14:05 -0700 Subject: [PATCH 05/11] fix error message --- action/deploy_recipe.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/action/deploy_recipe.py b/action/deploy_recipe.py index ccd0812..101e491 100644 --- a/action/deploy_recipe.py +++ b/action/deploy_recipe.py @@ -59,9 +59,9 @@ def main(): raise ValueError( f"{config_string} failed to parse to JSON. If you meant to pass a JSON string, " "please confirm that it is correctly formatted. If you meant to pass a filename, " - "please confirm this file exists at the given path. Note, this action is running " - f"where the workflow YAML file lives, which is: '{os.getcwd()}'. If you provide " - "a relative path, it must be relative to this location." + "please confirm this path exists. Note, pangeo-forge/deploy-recipe-action should " + "always be invoked after actions/checkout, therefore the provided path must be " + "given relative to the repo root." ) from e # log variables to stdout From df165b02c9ed507dd83498ef3f3d740026732ab6 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Mon, 20 Nov 2023 16:11:41 -0800 Subject: [PATCH 06/11] move test action setup into resuable workflow --- .github/workflows/setup.yaml | 27 +++++++++++++++++++++++++++ .github/workflows/test-action.yaml | 20 ++++---------------- 2 files changed, 31 insertions(+), 16 deletions(-) create mode 100644 .github/workflows/setup.yaml diff --git a/.github/workflows/setup.yaml b/.github/workflows/setup.yaml new file mode 100644 index 0000000..75b3af1 --- /dev/null +++ b/.github/workflows/setup.yaml @@ -0,0 +1,27 @@ +name: Reusable setup workflow for actions tests + +on: + workflow_call: + inputs: + recipes-version: + required: true + type: string + +jobs: + runs-on: ubuntu-latest + setup: + - name: "Clone test feedstock" + # Fetches test feedstock (containing example test recipes) from pangeo-forge-recipes + run: | + git clone --depth 1 --branch ${{ inputs.recipes-version }} https://github.com/pangeo-forge/pangeo-forge-recipes.git + - name: "Add requirements.txt" + # The clone step above gives us recipe modules and meta.yaml, but does not contain a requirements file, + # so we add that here, providing the action with the correct version of pangeo-forge-recipes to install + # in the container at action runtime. + run: | + echo "pangeo-forge-recipes==${{ inputs.recipes-version }}" > pangeo-forge-recipes/examples/feedstock/requirements.txt + - name: "Overwrite meta.yaml" + # The example feedstock contains multiple recipes, but we just want to test gpcp + run: | + grep -E 'recipes|gpcp' pangeo-forge-recipes/examples/feedstock/meta.yaml > temp.yaml \ + && mv temp.yaml pangeo-forge-recipes/examples/feedstock/meta.yaml diff --git a/.github/workflows/test-action.yaml b/.github/workflows/test-action.yaml index d5ff740..681e7bb 100644 --- a/.github/workflows/test-action.yaml +++ b/.github/workflows/test-action.yaml @@ -1,4 +1,4 @@ -name: Test action +name: Test action (inline config) on: push: @@ -16,21 +16,9 @@ jobs: recipes-version: ["0.10.3"] # , "main"] steps: - uses: actions/checkout@v4 - - name: "Clone test feedstock" - # Fetches test feedstock (containing example test recipes) from pangeo-forge-recipes - run: | - git clone --depth 1 --branch ${{ matrix.recipes-version }} https://github.com/pangeo-forge/pangeo-forge-recipes.git - - name: "Add requirements.txt" - # The clone step above gives us recipe modules and meta.yaml, but does not contain a requirements file, - # so we add that here, providing the action with the correct version of pangeo-forge-recipes to install - # in the container at action runtime. - run: | - echo "pangeo-forge-recipes==${{ matrix.recipes-version }}" > pangeo-forge-recipes/examples/feedstock/requirements.txt - - name: "Overwrite meta.yaml" - # The example feedstock contains multiple recipes, but we just want to test gpcp - run: | - grep -E 'recipes|gpcp' pangeo-forge-recipes/examples/feedstock/meta.yaml > temp.yaml \ - && mv temp.yaml pangeo-forge-recipes/examples/feedstock/meta.yaml + - uses: ./.github/workflows/setup.yaml + with: + recipes-version: ${{ matrix.recipes-version }} - name: "Deploy recipes" uses: ./ with: From b9935e8d3148aace42277522e685941774a11054 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Mon, 20 Nov 2023 16:20:13 -0800 Subject: [PATCH 07/11] add file config test --- .github/workflows/setup.yaml | 1 + .../workflows/test-action-file-config.yaml | 47 +++++++++++++++++++ ...on.yaml => test-action-inline-config.yaml} | 0 3 files changed, 48 insertions(+) create mode 100644 .github/workflows/test-action-file-config.yaml rename .github/workflows/{test-action.yaml => test-action-inline-config.yaml} (100%) diff --git a/.github/workflows/setup.yaml b/.github/workflows/setup.yaml index 75b3af1..bc20af7 100644 --- a/.github/workflows/setup.yaml +++ b/.github/workflows/setup.yaml @@ -10,6 +10,7 @@ on: jobs: runs-on: ubuntu-latest setup: + - uses: actions/checkout@v4 - name: "Clone test feedstock" # Fetches test feedstock (containing example test recipes) from pangeo-forge-recipes run: | diff --git a/.github/workflows/test-action-file-config.yaml b/.github/workflows/test-action-file-config.yaml new file mode 100644 index 0000000..9ad34d1 --- /dev/null +++ b/.github/workflows/test-action-file-config.yaml @@ -0,0 +1,47 @@ +name: Test action (file config) + +on: + push: + branches: ["main"] + pull_request: + branches: [ "*" ] + +jobs: + test: + name: (recipes@${{ matrix.recipes-version }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + recipes-version: ["0.10.3"] # , "main"] + steps: + - uses: actions/checkout@v4 + - uses: ./.github/workflows/setup.yaml + with: + recipes-version: ${{ matrix.recipes-version }} + - name: Write local-config.json + run: | + cat << EOF > ./local-config.json + { + "BaseCommand": { + "feedstock_subdir": "pangeo-forge-recipes/examples/feedstock" + }, + "Bake": { + "prune": true, + "bakery_class": "pangeo_forge_runner.bakery.local.LocalDirectBakery" + }, + "TargetStorage": { + "fsspec_class": "fsspec.implementations.local.LocalFileSystem", + "root_path": "./target" + }, + "InputCacheStorage": { + "fsspec_class": "fsspec.implementations.local.LocalFileSystem", + "root_path": "./cache" + } + } + EOF + - name: "Deploy recipes" + uses: ./ + with: + # select_recipe_by_label: true + pangeo_forge_runner_config: ./local-config.json diff --git a/.github/workflows/test-action.yaml b/.github/workflows/test-action-inline-config.yaml similarity index 100% rename from .github/workflows/test-action.yaml rename to .github/workflows/test-action-inline-config.yaml From 98c71f1b35ffefbcd0a5a9fa54ed3a314893c454 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Mon, 20 Nov 2023 16:35:27 -0800 Subject: [PATCH 08/11] both tests in one file --- .github/workflows/setup.yaml | 28 ------ .../workflows/test-action-file-config.yaml | 47 --------- .../workflows/test-action-inline-config.yaml | 43 -------- .github/workflows/test-action.yaml | 99 +++++++++++++++++++ 4 files changed, 99 insertions(+), 118 deletions(-) delete mode 100644 .github/workflows/setup.yaml delete mode 100644 .github/workflows/test-action-file-config.yaml delete mode 100644 .github/workflows/test-action-inline-config.yaml create mode 100644 .github/workflows/test-action.yaml diff --git a/.github/workflows/setup.yaml b/.github/workflows/setup.yaml deleted file mode 100644 index bc20af7..0000000 --- a/.github/workflows/setup.yaml +++ /dev/null @@ -1,28 +0,0 @@ -name: Reusable setup workflow for actions tests - -on: - workflow_call: - inputs: - recipes-version: - required: true - type: string - -jobs: - runs-on: ubuntu-latest - setup: - - uses: actions/checkout@v4 - - name: "Clone test feedstock" - # Fetches test feedstock (containing example test recipes) from pangeo-forge-recipes - run: | - git clone --depth 1 --branch ${{ inputs.recipes-version }} https://github.com/pangeo-forge/pangeo-forge-recipes.git - - name: "Add requirements.txt" - # The clone step above gives us recipe modules and meta.yaml, but does not contain a requirements file, - # so we add that here, providing the action with the correct version of pangeo-forge-recipes to install - # in the container at action runtime. - run: | - echo "pangeo-forge-recipes==${{ inputs.recipes-version }}" > pangeo-forge-recipes/examples/feedstock/requirements.txt - - name: "Overwrite meta.yaml" - # The example feedstock contains multiple recipes, but we just want to test gpcp - run: | - grep -E 'recipes|gpcp' pangeo-forge-recipes/examples/feedstock/meta.yaml > temp.yaml \ - && mv temp.yaml pangeo-forge-recipes/examples/feedstock/meta.yaml diff --git a/.github/workflows/test-action-file-config.yaml b/.github/workflows/test-action-file-config.yaml deleted file mode 100644 index 9ad34d1..0000000 --- a/.github/workflows/test-action-file-config.yaml +++ /dev/null @@ -1,47 +0,0 @@ -name: Test action (file config) - -on: - push: - branches: ["main"] - pull_request: - branches: [ "*" ] - -jobs: - test: - name: (recipes@${{ matrix.recipes-version }}) - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - recipes-version: ["0.10.3"] # , "main"] - steps: - - uses: actions/checkout@v4 - - uses: ./.github/workflows/setup.yaml - with: - recipes-version: ${{ matrix.recipes-version }} - - name: Write local-config.json - run: | - cat << EOF > ./local-config.json - { - "BaseCommand": { - "feedstock_subdir": "pangeo-forge-recipes/examples/feedstock" - }, - "Bake": { - "prune": true, - "bakery_class": "pangeo_forge_runner.bakery.local.LocalDirectBakery" - }, - "TargetStorage": { - "fsspec_class": "fsspec.implementations.local.LocalFileSystem", - "root_path": "./target" - }, - "InputCacheStorage": { - "fsspec_class": "fsspec.implementations.local.LocalFileSystem", - "root_path": "./cache" - } - } - EOF - - name: "Deploy recipes" - uses: ./ - with: - # select_recipe_by_label: true - pangeo_forge_runner_config: ./local-config.json diff --git a/.github/workflows/test-action-inline-config.yaml b/.github/workflows/test-action-inline-config.yaml deleted file mode 100644 index 681e7bb..0000000 --- a/.github/workflows/test-action-inline-config.yaml +++ /dev/null @@ -1,43 +0,0 @@ -name: Test action (inline config) - -on: - push: - branches: ["main"] - pull_request: - branches: [ "*" ] - -jobs: - test: - name: (recipes@${{ matrix.recipes-version }}) - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - recipes-version: ["0.10.3"] # , "main"] - steps: - - uses: actions/checkout@v4 - - uses: ./.github/workflows/setup.yaml - with: - recipes-version: ${{ matrix.recipes-version }} - - name: "Deploy recipes" - uses: ./ - with: - # select_recipe_by_label: true - pangeo_forge_runner_config: > - { - "BaseCommand": { - "feedstock_subdir": "pangeo-forge-recipes/examples/feedstock" - }, - "Bake": { - "prune": true, - "bakery_class": "pangeo_forge_runner.bakery.local.LocalDirectBakery" - }, - "TargetStorage": { - "fsspec_class": "fsspec.implementations.local.LocalFileSystem", - "root_path": "./target" - }, - "InputCacheStorage": { - "fsspec_class": "fsspec.implementations.local.LocalFileSystem", - "root_path": "./cache" - } - } diff --git a/.github/workflows/test-action.yaml b/.github/workflows/test-action.yaml new file mode 100644 index 0000000..028d024 --- /dev/null +++ b/.github/workflows/test-action.yaml @@ -0,0 +1,99 @@ +name: Test action + +on: + push: + branches: ["main"] + pull_request: + branches: [ "*" ] + +jobs: + test: + name: (recipes@${{ matrix.recipes-version }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + recipes-version: [ + # "main", + # "0.10.3", + "0.10.4", + ] + config: [ + "inline", + "file", + ] + steps: + - uses: actions/checkout@v4 + + # ----- Shared setup -------------------------------------------------------------- + - name: "Clone test feedstock" + # Fetches test feedstock (containing example test recipes) from pangeo-forge-recipes + run: | + git clone --depth 1 --branch ${{ matrix.recipes-version }} https://github.com/pangeo-forge/pangeo-forge-recipes.git + - name: "Add requirements.txt" + # The clone step above gives us recipe modules and meta.yaml, but does not contain a requirements file, + # so we add that here, providing the action with the correct version of pangeo-forge-recipes to install + # in the container at action runtime. + run: | + echo "pangeo-forge-recipes==${{ matric.recipes-version }}" > pangeo-forge-recipes/examples/feedstock/requirements.txt + - name: "Overwrite meta.yaml" + # The example feedstock contains multiple recipes, but we just want to test gpcp + run: | + grep -E 'recipes|gpcp' pangeo-forge-recipes/examples/feedstock/meta.yaml > temp.yaml \ + && mv temp.yaml pangeo-forge-recipes/examples/feedstock/meta.yaml + + # ----- File based config --------------------------------------------------------- + - name: Write local-config.json + if: matrix.config == 'file' + run: | + cat << EOF > ./local-config.json + { + "BaseCommand": { + "feedstock_subdir": "pangeo-forge-recipes/examples/feedstock" + }, + "Bake": { + "prune": true, + "bakery_class": "pangeo_forge_runner.bakery.local.LocalDirectBakery" + }, + "TargetStorage": { + "fsspec_class": "fsspec.implementations.local.LocalFileSystem", + "root_path": "./target" + }, + "InputCacheStorage": { + "fsspec_class": "fsspec.implementations.local.LocalFileSystem", + "root_path": "./cache" + } + } + EOF + - name: "Deploy recipes" + if: matrix.config == 'file' + uses: ./ + with: + # select_recipe_by_label: true + pangeo_forge_runner_config: ./local-config.json + + + # ---- Inline config -------------------------------------------------------------- + - name: "Deploy recipes" + if: matrix.config == 'inline' + uses: ./ + with: + # select_recipe_by_label: true + pangeo_forge_runner_config: > + { + "BaseCommand": { + "feedstock_subdir": "pangeo-forge-recipes/examples/feedstock" + }, + "Bake": { + "prune": true, + "bakery_class": "pangeo_forge_runner.bakery.local.LocalDirectBakery" + }, + "TargetStorage": { + "fsspec_class": "fsspec.implementations.local.LocalFileSystem", + "root_path": "./target" + }, + "InputCacheStorage": { + "fsspec_class": "fsspec.implementations.local.LocalFileSystem", + "root_path": "./cache" + } + } From 467b350039ccc2fa76a420f657b58acb26f7a92e Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Mon, 20 Nov 2023 16:36:30 -0800 Subject: [PATCH 09/11] fix typo --- .github/workflows/test-action.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-action.yaml b/.github/workflows/test-action.yaml index 028d024..feb6514 100644 --- a/.github/workflows/test-action.yaml +++ b/.github/workflows/test-action.yaml @@ -35,7 +35,7 @@ jobs: # so we add that here, providing the action with the correct version of pangeo-forge-recipes to install # in the container at action runtime. run: | - echo "pangeo-forge-recipes==${{ matric.recipes-version }}" > pangeo-forge-recipes/examples/feedstock/requirements.txt + echo "pangeo-forge-recipes==${{ matrix.recipes-version }}" > pangeo-forge-recipes/examples/feedstock/requirements.txt - name: "Overwrite meta.yaml" # The example feedstock contains multiple recipes, but we just want to test gpcp run: | From e37dd0e717ecb6b38b267ee15a8768c4ff3a556b Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Tue, 21 Nov 2023 10:34:40 -0800 Subject: [PATCH 10/11] mock test coverage for file config --- .github/workflows/test-action.yaml | 2 +- tests/test_main.py | 61 ++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test-action.yaml b/.github/workflows/test-action.yaml index feb6514..806dbb6 100644 --- a/.github/workflows/test-action.yaml +++ b/.github/workflows/test-action.yaml @@ -8,7 +8,7 @@ on: jobs: test: - name: (recipes@${{ matrix.recipes-version }}) + name: (recipes@${{ matrix.recipes-version }}, config=${{ matrix.config }}) runs-on: ubuntu-latest strategy: fail-fast: false diff --git a/tests/test_main.py b/tests/test_main.py index effc35d..835a358 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -24,8 +24,23 @@ def select_recipe_by_label(request): return request.param +@pytest.fixture(params=["inline", "file", "broken_inline", "broken_file"]) +def pangeo_forge_runner_config(request, tmp_path_factory: pytest.TempPathFactory): + if "inline" in request.param: + return "{}" if not "broken" in request.param else "{broken: json}" + elif "file" in request.param: + if not "broken" in request.param: + tmp_config_dir = tmp_path_factory.mktemp("config") + outpath = tmp_config_dir / "config.json" + with open(tmp_config_dir / "config.json", mode="w") as f: + json.dump({}, f) + return outpath.absolute().as_posix() + else: + return "non/existent/path.json" + + @pytest.fixture -def env(select_recipe_by_label, head_ref): +def env(select_recipe_by_label, head_ref, pangeo_forge_runner_config): return { "GITHUB_REPOSITORY": "my/repo", "GITHUB_API_URL": "https://api.github.com", @@ -36,7 +51,7 @@ def env(select_recipe_by_label, head_ref): "GITHUB_REPOSITORY_ID": "1234567890", "GITHUB_RUN_ID": "0987654321", "GITHUB_RUN_ATTEMPT": "1", - "INPUT_PANGEO_FORGE_RUNNER_CONFIG": '{}', + "INPUT_PANGEO_FORGE_RUNNER_CONFIG": pangeo_forge_runner_config, "INPUT_SELECT_RECIPE_BY_LABEL": select_recipe_by_label, } @@ -101,6 +116,14 @@ def mock_tempfile_name(): return "mock-temp-file.json" +def get_config_asdict(config: str) -> dict: + """Config could be a JSON file path or JSON string, either way load it to dict.""" + if os.path.exists(config): + with open(config) as f: + return json.load(f) + return json.loads(config) + + @patch("deploy_recipe.os.listdir") @patch("deploy_recipe.subprocess.run") @patch("deploy_recipe.requests.get") @@ -134,24 +157,46 @@ def test_main( listdir_return_value, pip_install_raises = listdir_return_value_with_pip_install_raises listdir.return_value = listdir_return_value - if pip_install_raises: - config: dict = json.loads(env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"]) - config.update({"BaseCommand": {"feedstock_subdir": "broken-requirements-feedstock"}}) - env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"] = json.dumps(config) + config_is_broken = ( + env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"] == "{broken: json}" + or env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"] == "non/existent/path.json" + ) + + if pip_install_raises and not config_is_broken: + update_with = {"BaseCommand": {"feedstock_subdir": "broken-requirements-feedstock"}} + config = get_config_asdict(env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"]) + config.update(update_with) + + if os.path.exists(env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"]): + with open(env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"], "w") as f: + json.dump(config, f) + else: + env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"] = json.dumps(config) with patch.dict(os.environ, env): - if "broken-requirements-feedstock" in env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"]: + if ( + not config_is_broken + and "broken-requirements-feedstock" in str( + get_config_asdict(env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"]) + ) + ): with pytest.raises(ValueError): main() # if pip install fails, we should bail early & never invoke `bake`. re: `call_args_list`: # https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock.call_args_list for call in [args[0][0] for args in subprocess_run.call_args_list]: assert "bake" not in call + elif env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"] == "{broken: json}": + with pytest.raises(ValueError): + main() + elif env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"] == "non/existent/path.json": + with pytest.raises(ValueError): + main() else: main() if any([path.endswith("requirements.txt") for path in listdir_return_value]): - config = json.loads(env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"]) + config = get_config_asdict(env["INPUT_PANGEO_FORGE_RUNNER_CONFIG"]) feedstock_subdir = ( config["BaseCommand"]["feedstock-subdir"] if "BaseCommand" in config From fe566537f31963b46229de69e50fbb95de76666b Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Tue, 21 Nov 2023 10:45:58 -0800 Subject: [PATCH 11/11] jsondecodeerror could happen with either file or inline --- action/deploy_recipe.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/action/deploy_recipe.py b/action/deploy_recipe.py index 8d367aa..0e91aca 100644 --- a/action/deploy_recipe.py +++ b/action/deploy_recipe.py @@ -55,24 +55,24 @@ def main(): # parse config print(f"pangeo-forge-runner-config provided as {config_string}") - if os.path.exists(config_string): - # we allow local paths pointing to json files - print(f"Loading json from file '{config_string}'...") - with open(config_string) as f: - config = json.load(f) - else: - # or json strings passed inline in the workflow yaml - print(f"{config_string} does not exist as a file. Loading json from string...") - try: + try: + if os.path.exists(config_string): + # we allow local paths pointing to json files + print(f"Loading json from file '{config_string}'...") + with open(config_string) as f: + config = json.load(f) + else: + # or json strings passed inline in the workflow yaml + print(f"{config_string} does not exist as a file. Loading json from string...") config = json.loads(config_string) - except json.JSONDecodeError as e: - raise ValueError( - f"{config_string} failed to parse to JSON. If you meant to pass a JSON string, " - "please confirm that it is correctly formatted. If you meant to pass a filename, " - "please confirm this path exists. Note, pangeo-forge/deploy-recipe-action should " - "always be invoked after actions/checkout, therefore the provided path must be " - "given relative to the repo root." - ) from e + except json.JSONDecodeError as e: + raise ValueError( + f"{config_string} failed to parse to JSON. If you meant to pass a JSON string, " + "please confirm that it is correctly formatted. If you meant to pass a filename, " + "please confirm this path exists. Note, pangeo-forge/deploy-recipe-action should " + "always be invoked after actions/checkout, therefore the provided path must be " + "given relative to the repo root." + ) from e # log variables to stdout print(f"{head_ref = }")