From 0f8a047830942a872f62460b9fdac3c49c514962 Mon Sep 17 00:00:00 2001 From: Sean Quinlan <1011062+sbquinlan@users.noreply.github.com> Date: Thu, 26 Oct 2023 17:16:37 -0700 Subject: [PATCH 1/7] Restore optional cache target --- pangeo_forge_runner/commands/bake.py | 10 +++------- pangeo_forge_runner/storage.py | 9 +++++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/pangeo_forge_runner/commands/bake.py b/pangeo_forge_runner/commands/bake.py index f9638f9b..96f74627 100644 --- a/pangeo_forge_runner/commands/bake.py +++ b/pangeo_forge_runner/commands/bake.py @@ -207,11 +207,9 @@ def start(self): ), } - cache_target = input_cache_storage.get_forge_target(job_name=self.job_name) - if cache_target: + if not input_cache_storage.is_default(): + cache_target = input_cache_storage.get_forge_target(job_name=self.job_name) injection_values |= {"INPUT_CACHE_STORAGE": cache_target} - print(injection_values) - print(injection_specs) feedstock = Feedstock( Path(checkout_dir) / self.feedstock_subdir, @@ -285,9 +283,7 @@ def start(self): ("cache", "metadata"), (input_cache_storage, metadata_cache_storage), ): - # `.root_path` is an empty string by default, so if the user has not setup this - # optional storage type in config, this block is skipped. - if optional_storage.root_path: + if not optional_storage.is_default(): setattr( recipe.storage_config, attrname, diff --git a/pangeo_forge_runner/storage.py b/pangeo_forge_runner/storage.py index b988497d..aa17576a 100644 --- a/pangeo_forge_runner/storage.py +++ b/pangeo_forge_runner/storage.py @@ -44,6 +44,15 @@ class StorageTargetConfig(LoggingConfigurable): """, ) + def is_default(self): + """ + Return if `root_path` is an empty string + + `.root_path` is an empty string by default. For optional storage targets, + this is used to mean it's unconfigured. + """ + return self.fsspec_class == AbstractFileSystem and not self.root_path + def get_forge_target(self, job_name: str): """ Return correct pangeo-forge-recipes Target From 7fad4a3409afefecc0794d479b323709d0139bec Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 27 Oct 2023 00:19:47 +0000 Subject: [PATCH 2/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pangeo_forge_runner/commands/bake.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pangeo_forge_runner/commands/bake.py b/pangeo_forge_runner/commands/bake.py index 96f74627..94773e7e 100644 --- a/pangeo_forge_runner/commands/bake.py +++ b/pangeo_forge_runner/commands/bake.py @@ -208,7 +208,9 @@ def start(self): } if not input_cache_storage.is_default(): - cache_target = input_cache_storage.get_forge_target(job_name=self.job_name) + cache_target = input_cache_storage.get_forge_target( + job_name=self.job_name + ) injection_values |= {"INPUT_CACHE_STORAGE": cache_target} feedstock = Feedstock( From 40b316f59d5d6f9d03580ef89758bec5dfcc99c0 Mon Sep 17 00:00:00 2001 From: Sean Quinlan <1011062+sbquinlan@users.noreply.github.com> Date: Wed, 15 Nov 2023 22:19:22 -0800 Subject: [PATCH 3/7] Add test to test_bake --- tests/unit/test_bake.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_bake.py b/tests/unit/test_bake.py index 9ea12079..4e76222a 100644 --- a/tests/unit/test_bake.py +++ b/tests/unit/test_bake.py @@ -102,19 +102,21 @@ def test_container_name_validation(container_image, raises): @pytest.mark.parametrize( - ("recipe_id", "expected_error", "custom_job_name"), + ("recipe_id", "expected_error", "custom_job_name", "no_input_cache"), ( - [None, None, None], - ["gpcp-from-gcs", None, None], + [None, None, None, False], + ["gpcp-from-gcs", None, None, False], [ "invalid_recipe_id", "ValueError: self.recipe_id='invalid_recipe_id' not in ['gpcp-from-gcs']", None, + False ], - [None, None, "special-name-for-job"], + [None, None, "special-name-for-job", False], + [None, None, None, True], ), ) -def test_gpcp_bake(minio, recipe_id, expected_error, custom_job_name): +def test_gpcp_bake(minio, recipe_id, expected_error, custom_job_name, no_input_cache): fsspec_args = { "key": minio["username"], "secret": minio["password"], @@ -143,6 +145,12 @@ def test_gpcp_bake(minio, recipe_id, expected_error, custom_job_name): }, } + if no_input_cache: + config["InputCacheStorage"] = { + "fsspec_class": "AbstractFileSystem", + "fsspec_args": {}, + "root_path": "", + } if recipe_id: config["Bake"].update({"recipe_id": recipe_id}) if custom_job_name: From ae275fa692fb62d33af8679191243cc732b7db85 Mon Sep 17 00:00:00 2001 From: Sean Quinlan <1011062+sbquinlan@users.noreply.github.com> Date: Thu, 16 Nov 2023 20:41:27 -0800 Subject: [PATCH 4/7] Flying in the dark here, not sure what the test requirements are --- tests/unit/test_bake.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_bake.py b/tests/unit/test_bake.py index 4e76222a..50d32cbd 100644 --- a/tests/unit/test_bake.py +++ b/tests/unit/test_bake.py @@ -110,7 +110,7 @@ def test_container_name_validation(container_image, raises): "invalid_recipe_id", "ValueError: self.recipe_id='invalid_recipe_id' not in ['gpcp-from-gcs']", None, - False + False, ], [None, None, "special-name-for-job", False], [None, None, None, True], @@ -147,7 +147,7 @@ def test_gpcp_bake(minio, recipe_id, expected_error, custom_job_name, no_input_c if no_input_cache: config["InputCacheStorage"] = { - "fsspec_class": "AbstractFileSystem", + "fsspec_class": "fsspec.AbstractFileSystem", "fsspec_args": {}, "root_path": "", } @@ -221,7 +221,6 @@ def test_gpcp_bake(minio, recipe_id, expected_error, custom_job_name, no_input_c ) # --prune prunes to two time steps by default, so we expect 2 items here assert len(gpcp.precip) == 2 - print(gpcp) # `mc` isn't the best way, but we want to display all the files in our minio with tempfile.TemporaryDirectory() as mcd: From 620c3c10196f33296e884bc3fa638dce9f599038 Mon Sep 17 00:00:00 2001 From: Sean Quinlan <1011062+sbquinlan@users.noreply.github.com> Date: Thu, 16 Nov 2023 20:52:21 -0800 Subject: [PATCH 5/7] Add case for 0.9.4 failure --- tests/unit/test_bake.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_bake.py b/tests/unit/test_bake.py index 50d32cbd..9d1ad2a3 100644 --- a/tests/unit/test_bake.py +++ b/tests/unit/test_bake.py @@ -184,7 +184,9 @@ def test_gpcp_bake(minio, recipe_id, expected_error, custom_job_name, no_input_c if expected_error: assert proc.returncode == 1 stdout[-1] == expected_error - + elif no_input_cache and recipe_version_ref == "0.9.x": + # no_input_cache is only supported in 0.10.x and above + assert proc.returncode == 1 else: assert proc.returncode == 0 From 7f8525b8ad702778ce4b29472e2e25dd568ab992 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 21 Nov 2023 23:02:53 +0000 Subject: [PATCH 6/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit/test_bake.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_bake.py b/tests/unit/test_bake.py index 9332fefb..7c032d1f 100644 --- a/tests/unit/test_bake.py +++ b/tests/unit/test_bake.py @@ -132,7 +132,12 @@ def recipes_version_ref(request): ), ) def test_gpcp_bake( - minio, recipe_id, expected_error, custom_job_name, no_input_cache, recipes_version_ref + minio, + recipe_id, + expected_error, + custom_job_name, + no_input_cache, + recipes_version_ref, ): if recipes_version_ref == "0.9.x-dictobj" or ( recipes_version_ref == "0.10.x-dictobj" and recipe_id From 79fb5df4c999cc8702d84524b4090cde70459c40 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Tue, 21 Nov 2023 15:09:41 -0800 Subject: [PATCH 7/7] fix typo from merge --- tests/unit/test_bake.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_bake.py b/tests/unit/test_bake.py index 7c032d1f..5281b908 100644 --- a/tests/unit/test_bake.py +++ b/tests/unit/test_bake.py @@ -208,7 +208,7 @@ def test_gpcp_bake( if expected_error: assert proc.returncode == 1 stdout[-1] == expected_error - elif no_input_cache and recipe_version_ref == "0.9.x": + elif no_input_cache and recipes_version_ref == "0.9.x": # no_input_cache is only supported in 0.10.x and above assert proc.returncode == 1 else: