From 3cd642c078e3da985a7e82745cb4c8b41076a47f Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 24 Feb 2023 17:57:41 -0800 Subject: [PATCH 01/18] Support setting arbitrary logging config --- pangeo_forge_runner/commands/base.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/pangeo_forge_runner/commands/base.py b/pangeo_forge_runner/commands/base.py index 63e8d492..30c29593 100644 --- a/pangeo_forge_runner/commands/base.py +++ b/pangeo_forge_runner/commands/base.py @@ -7,7 +7,7 @@ from pythonjsonlogger import jsonlogger from repo2docker import contentproviders -from traitlets import Bool, Instance, List, Unicode +from traitlets import Bool, Dict, Instance, List, Unicode from traitlets.config import Application # Common aliases we want to support in *all* commands @@ -41,6 +41,21 @@ class BaseCommand(Application): log_level = logging.INFO + logging_config = Dict( + {}, + config=True, + help=""" + Logging configuration for this python application. + + When set, this value is passed to logging.config.dictConfig, + and can be used to configure how logs *throughout the application* + are handled, not just for logs from this application alone. + + See https://docs.python.org/3/library/logging.config.html#logging.config.dictConfig + for more details. + """, + ) + repo = Unicode( "", config=True, @@ -199,6 +214,13 @@ def initialize(self, argv=None): # Load traitlets config from a config file if present self.load_config_file(self.config_file) + # Allow arbitrary logging config if set + # We do this first up so any custom logging we set up ourselves + # is not affected, as by default dictConfig will replace all + # existing config. + if self.logging_config: + logging.config.dictConfig(self.logging_config) + # The application communicates with the outside world via # stdout, and we structure this communication via logging. # So let's setup the default logger to log to stdout, rather From e6a56f1dc32c9aa5ed0726685a21c4174f9ee116 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Tue, 1 Aug 2023 13:42:48 -0700 Subject: [PATCH 02/18] Create release.yaml --- .github/workflows/release.yaml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/workflows/release.yaml diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml new file mode 100644 index 00000000..b5ff01d3 --- /dev/null +++ b/.github/workflows/release.yaml @@ -0,0 +1,30 @@ +# Same as https://github.com/pangeo-forge/pangeo-forge-recipes/blob/main/.github/workflows/release.yaml + +name: Release Python Package + +on: + release: + types: [created] + +jobs: + deploy: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: '3.x' + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install --upgrade setuptools setuptools-scm wheel twine toml + - name: Build and publish + env: + TWINE_USERNAME: "__token__" + TWINE_PASSWORD: ${{ secrets.PYPI_TOKEN }} + run: | + python setup.py sdist bdist_wheel + python setup.py --version + twine check dist/* + twine upload dist/* From 87ec8b85f287b2f2edfa87694126395c71471191 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Thu, 3 Aug 2023 15:09:47 -0700 Subject: [PATCH 03/18] upgrade flink operator to 1.5.0 --- .github/workflows/flink.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/flink.yaml b/.github/workflows/flink.yaml index 81a04284..cd71dccb 100644 --- a/.github/workflows/flink.yaml +++ b/.github/workflows/flink.yaml @@ -39,7 +39,7 @@ jobs: - name: Setup FlinkOperator run: | - FLINK_OPERATOR_VERSION=1.3.0 + FLINK_OPERATOR_VERSION=1.5.0 helm repo add flink-operator-repo https://downloads.apache.org/flink/flink-kubernetes-operator-${FLINK_OPERATOR_VERSION} helm install flink-kubernetes-operator flink-operator-repo/flink-kubernetes-operator --wait From 231e35df6d49e11166ac65b6333e34b8f4ea72c7 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Thu, 3 Aug 2023 16:45:48 -0700 Subject: [PATCH 04/18] print sterr if flink proc fails --- tests/integration/test_flink.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_flink.py b/tests/integration/test_flink.py index 631c44e3..519c2493 100644 --- a/tests/integration/test_flink.py +++ b/tests/integration/test_flink.py @@ -48,9 +48,12 @@ def test_flink_bake(minio): "-f", f.name, ] - proc = subprocess.run(cmd) + proc = subprocess.run(cmd, capture_output=True) - assert proc.returncode == 0 + if proc.returncode != 0: + print(proc.stderr.decode()) + + assert proc.returncode == 0 # bail if proc did not succeed # We should have some kinda 'has this completed?' check here # Instead, I just wait for 3min From 0f100acb0eac02190b1582700a3b5beb57cbb168 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Thu, 3 Aug 2023 16:56:36 -0700 Subject: [PATCH 05/18] revert stderr print, capture_output=True gives us the trace. --- tests/integration/test_flink.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/integration/test_flink.py b/tests/integration/test_flink.py index 519c2493..a1c9fab7 100644 --- a/tests/integration/test_flink.py +++ b/tests/integration/test_flink.py @@ -50,10 +50,7 @@ def test_flink_bake(minio): ] proc = subprocess.run(cmd, capture_output=True) - if proc.returncode != 0: - print(proc.stderr.decode()) - - assert proc.returncode == 0 # bail if proc did not succeed + assert proc.returncode == 0 # We should have some kinda 'has this completed?' check here # Instead, I just wait for 3min From 6babb04510ffc727766d7aee1aa45c0458d8588a Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Thu, 3 Aug 2023 16:57:09 -0700 Subject: [PATCH 06/18] use beam-refactor ref for flink test --- tests/integration/test_flink.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_flink.py b/tests/integration/test_flink.py index a1c9fab7..1ea61e22 100644 --- a/tests/integration/test_flink.py +++ b/tests/integration/test_flink.py @@ -44,7 +44,7 @@ def test_flink_bake(minio): "--repo", "https://github.com/pangeo-forge/gpcp-feedstock.git", "--ref", - "2cde04745189665a1f5a05c9eae2a98578de8b7f", + "beam-refactor", "-f", f.name, ] From db67f2232a49087ff3885dc3bc01aa1cace5b4d8 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Thu, 3 Aug 2023 17:02:32 -0700 Subject: [PATCH 07/18] test flink against pforgetest repo --- tests/integration/test_flink.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_flink.py b/tests/integration/test_flink.py index 1ea61e22..65202afd 100644 --- a/tests/integration/test_flink.py +++ b/tests/integration/test_flink.py @@ -42,7 +42,7 @@ def test_flink_bake(minio): "pangeo-forge-runner", "bake", "--repo", - "https://github.com/pangeo-forge/gpcp-feedstock.git", + "https://github.com/pforgetest/gpcp-from-gcs-feedstock.git", "--ref", "beam-refactor", "-f", From 1f4af180c789e296102f27f0510eaecb7dd3e4a6 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 18 Aug 2023 23:21:53 -0700 Subject: [PATCH 08/18] Cleanup how we pick beam or no-beam versions of recipes to test - Explicitly find the version we care about in a way that works for local installs too - current code doesn't clearly parse output of local .dev installs of pangeo-forge-recipes. - The fixture made it appear to be a general pattern in use across repos - but this tagging ref is actually only true for a specific repo. So move the parsing code close to where it is used, rather than in a more general place. --- tests/conftest.py | 15 --------------- tests/integration/test_dataflow_integration.py | 15 +++++++++++---- tests/unit/test_bake.py | 17 ++++++++++++----- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index a67ba59d..66049dfa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -57,18 +57,3 @@ def minio(local_ip): proc.wait() assert proc.returncode == 0 - - -@pytest.fixture -def recipes_version_ref(): - # FIXME: recipes version matrix is currently determined by github workflows matrix - # in the future, it should be set by pangeo-forge-runner venv feature? - pip_list = subprocess.check_output("pip list".split()).decode("utf-8").splitlines() - recipes_version = [ - p.split()[-1] for p in pip_list if p.startswith("pangeo-forge-recipes") - ][0] - # the recipes_version is a 3-element semantic version of form `0.A.B` where A is either minor - # version `9` or `10`. the test feedstock (pforgetest/gpcp-from-gcs-feedstock) has tags for - # each of these minor versions, of the format `0.A.x`, so we translate the installed version - # of pangeo-forge-recipes to one of the valid tags (either `0.9.x` or `0.10.x`) here. - return f"0.{recipes_version.split('.')[1]}.x" diff --git a/tests/integration/test_dataflow_integration.py b/tests/integration/test_dataflow_integration.py index 20a4f146..9b5aa54b 100644 --- a/tests/integration/test_dataflow_integration.py +++ b/tests/integration/test_dataflow_integration.py @@ -2,12 +2,19 @@ import subprocess import tempfile import time +from importlib.metadata import version import pytest import xarray as xr +from packaging.version import parse as parse_version -def test_dataflow_integration(recipes_version_ref): +def test_dataflow_integration(): + pfr_version = parse_version(version("pangeo-forge-recipes")) + if pfr_version >= parse_version("0.10"): + recipe_version_ref = "0.10.x" + else: + recipe_version_ref = "0.9.x" bucket = "gs://pangeo-forge-runner-ci-testing" config = { "Bake": { @@ -40,7 +47,7 @@ def test_dataflow_integration(recipes_version_ref): "--ref", # in the test feedstock, tags are named for the recipes version # which was used to write the recipe module - recipes_version_ref, + recipe_version_ref, "--json", "-f", f.name, @@ -93,8 +100,8 @@ def test_dataflow_integration(recipes_version_ref): # open the generated dataset with xarray! target_path = config["TargetStorage"]["root_path"].format(job_name=job_name) - if recipes_version_ref == "0.10.x": - # in pangeo-forge-recipes>=0.10.0, an additional `StoreToZarr.store_name` kwarg + if pfr_version >= parse_version("0.10"): + # in pangeo-forge-eecipes>=0.10.0, an additional `StoreToZarr.store_name` kwarg # is appended to the formatted root path at execution time. for ref `0.10.x`, # the value of that kwarg is "gpcp", so we append that here. target_path += "/gpcp" diff --git a/tests/unit/test_bake.py b/tests/unit/test_bake.py index 6f3752bf..08defa0f 100644 --- a/tests/unit/test_bake.py +++ b/tests/unit/test_bake.py @@ -2,9 +2,11 @@ import re import subprocess import tempfile +from importlib.metadata import version import pytest import xarray as xr +from packaging.version import parse as parse_version from pangeo_forge_runner.commands.bake import Bake @@ -50,9 +52,7 @@ def test_job_name_validation(job_name, raises): [None, None, "special-name-for-job"], ), ) -def test_gpcp_bake( - minio, recipe_id, expected_error, custom_job_name, recipes_version_ref -): +def test_gpcp_bake(minio, recipe_id, expected_error, custom_job_name): fsspec_args = { "key": minio["username"], "secret": minio["password"], @@ -86,6 +86,12 @@ def test_gpcp_bake( if custom_job_name: config["Bake"].update({"job_name": custom_job_name}) + pfr_version = parse_version(version("pangeo-forge-recipes")) + if pfr_version >= parse_version("0.10"): + recipe_version_ref = "0.10.x" + else: + recipe_version_ref = "0.9.x" + with tempfile.NamedTemporaryFile("w", suffix=".json") as f: json.dump(config, f) f.flush() @@ -97,7 +103,7 @@ def test_gpcp_bake( "--ref", # in the test feedstock, tags are named for the recipes version # which was used to write the recipe module - recipes_version_ref, + recipe_version_ref, "--json", "-f", f.name, @@ -126,7 +132,8 @@ def test_gpcp_bake( # root path itself. This is a compatibility break vs the previous # versions of pangeo-forge-recipes. https://github.com/pangeo-forge/pangeo-forge-recipes/pull/495 # has more information - if recipes_version_ref == "0.10.x": + + if pfr_version >= parse_version("0.10"): zarr_store_path = config["TargetStorage"]["root_path"] + "gpcp/" else: zarr_store_path = config["TargetStorage"]["root_path"] From c8b6844f3f520ecadba15b08ed63161ee9933554 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Sat, 19 Aug 2023 11:24:35 -0700 Subject: [PATCH 09/18] Throw error and exit if config file isn't readable Fixes https://github.com/pangeo-forge/pangeo-forge-runner/issues/73 --- pangeo_forge_runner/commands/base.py | 13 +++++++++++-- tests/unit/test_expand_meta.py | 10 ++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pangeo_forge_runner/commands/base.py b/pangeo_forge_runner/commands/base.py index 30c29593..5201dbca 100644 --- a/pangeo_forge_runner/commands/base.py +++ b/pangeo_forge_runner/commands/base.py @@ -211,8 +211,17 @@ def json_excepthook(self, etype, evalue, traceback): def initialize(self, argv=None): super().initialize(argv) - # Load traitlets config from a config file if present - self.load_config_file(self.config_file) + # Load traitlets config from a config file if passed + if self.config_file: + if os.path.exists(self.config_file): + # Throw an explicit error and exit if config file isn't present + print( + f"Could not read config from file {self.config_file}. Make sure it exists and is readable", + file=sys.stderr, + ) + sys.exit(1) + + self.load_config_file(self.config_file) # Allow arbitrary logging config if set # We do this first up so any custom logging we set up ourselves diff --git a/tests/unit/test_expand_meta.py b/tests/unit/test_expand_meta.py index cb5ee740..e80b3cc5 100644 --- a/tests/unit/test_expand_meta.py +++ b/tests/unit/test_expand_meta.py @@ -1,4 +1,6 @@ import json +import os +import secrets import subprocess invocations = [ @@ -75,3 +77,11 @@ def test_expand_meta_no_json(): out = subprocess.check_output(cmd, encoding="utf-8") last_line = out.splitlines()[-1] assert json.loads(last_line) == invocation["meta"] + + +def test_missing_config_file(): + non_existent_path = secrets.token_hex() + ".py" + assert not os.path.exists(non_existent_path) + cmd = ["pangeo-forge-runner", "expand-meta", "--config", non_existent_path] + proc = subprocess.run(cmd, encoding="utf-8") + assert proc.returncode == 1 From df635f2088b5d28dc3e1af382032efd8f27b1955 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Fri, 25 Aug 2023 14:17:58 -0700 Subject: [PATCH 10/18] use pypi trusted publisher --- .github/workflows/release.yaml | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index b5ff01d3..932da355 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -1,6 +1,4 @@ -# Same as https://github.com/pangeo-forge/pangeo-forge-recipes/blob/main/.github/workflows/release.yaml - -name: Release Python Package +name: Release on: release: @@ -9,6 +7,10 @@ on: jobs: deploy: runs-on: ubuntu-latest + environment: + name: release + permissions: + id-token: write steps: - uses: actions/checkout@v2 - name: Set up Python @@ -19,12 +21,13 @@ jobs: run: | python -m pip install --upgrade pip python -m pip install --upgrade setuptools setuptools-scm wheel twine toml - - name: Build and publish - env: - TWINE_USERNAME: "__token__" - TWINE_PASSWORD: ${{ secrets.PYPI_TOKEN }} + - name: Build run: | python setup.py sdist bdist_wheel python setup.py --version twine check dist/* - twine upload dist/* + ls -l dist + - name: Publish + uses: pypa/gh-action-pypi-publish@release/v1 + if: startsWith(github.ref, 'refs/tags/') + From 014e0bd4b46e0a9db54fefc4ef92295ee25b9f87 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Fri, 25 Aug 2023 14:36:47 -0700 Subject: [PATCH 11/18] dynamic versioning with setuptools_scm --- .gitignore | 1 + pyproject.toml | 11 +++++++++++ setup.py | 2 -- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index b6e47617..babaf104 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,7 @@ share/python-wheels/ .installed.cfg *.egg MANIFEST +_version.py # PyInstaller # Usually these files are written by a python script from a template diff --git a/pyproject.toml b/pyproject.toml index 669ca4fe..c53624c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,14 @@ +[build-system] +requires = ["setuptools>=45", "setuptools_scm[toml]>=6.2"] + +[project] +name = "pangeo-forge-runner" +dynamic = ["version"] + +[tool.setuptools_scm] +write_to = "pangeo_forge_runner/_version.py" +write_to_template = "__version__ = '{version}'" + [tool.isort] # Prevent isort & black from fighting each otherd profile = "black" diff --git a/setup.py b/setup.py index 7dbe2377..8045847a 100644 --- a/setup.py +++ b/setup.py @@ -4,13 +4,11 @@ readme = f.read() setup( - name="pangeo-forge-runner", description="Commandline tool to manage pangeo-forge feedstocks", long_description=readme, long_description_content_type="text/markdown", author="Yuvi Panda", author_email="yuvipanda@gmail.com", - version="0.7.2", packages=find_packages(), python_requires=">=3.9", install_requires=[ From 400f0b08f90f2ff73a6677e57fa2ab1bd754429c Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Fri, 25 Aug 2023 14:44:04 -0700 Subject: [PATCH 12/18] fix docs build --- docs/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/requirements.txt b/docs/requirements.txt index a79c6e56..f2b21713 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -2,5 +2,5 @@ autodoc-traits myst-parser pre-commit pydata-sphinx-theme -sphinx>=1.7 +sphinx>=1.7,<7.2 # https://github.com/sphinx-doc/sphinx/issues/11631 sphinx-copybutton From 1c8ba25731f015dcad1f8ec5e40dc9c82599e762 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Mon, 28 Aug 2023 22:14:56 -0700 Subject: [PATCH 13/18] trigger release on tag --- .github/workflows/release.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 932da355..2160321f 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -1,8 +1,8 @@ name: Release on: - release: - types: [created] + push: + tags: ["**"] jobs: deploy: From 157ae47fd9d47f040f3d393ece8d71ff92199662 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Tue, 29 Aug 2023 00:28:39 -0700 Subject: [PATCH 14/18] release docs --- docs/development/release.md | 17 +++++++++++++++++ docs/index.md | 8 ++++++++ 2 files changed, 25 insertions(+) create mode 100644 docs/development/release.md diff --git a/docs/development/release.md b/docs/development/release.md new file mode 100644 index 00000000..5b0034c1 --- /dev/null +++ b/docs/development/release.md @@ -0,0 +1,17 @@ +# Release + +Releases are automated by the `release.yaml` GitHub Workflow, +which is triggered by tag events. + +To cut a new release, those with push permissions to the repo, may run: + +```console +git tag $VERSION +git push origin --tags +``` + +Where `$VERSION` is a three-element, dot-delimited semantic version of the form +`v{MAJOR}.{MINOR}.{PATCH}`, which is appropriately incremented from the prior tag. + +And `origin` is assumed to be the remote corresponding to +`pangeo-forge/pangeo-forge-runner`. diff --git a/docs/index.md b/docs/index.md index 557a2b7a..ac5aba16 100644 --- a/docs/index.md +++ b/docs/index.md @@ -24,3 +24,11 @@ tutorial/flink reference/index ``` + +## Development + +```{toctree} +:maxdepth: 2 + +development/release +``` From da8901c578f421d8393074d771273451e1ee9e5c Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Fri, 1 Sep 2023 11:26:45 -0700 Subject: [PATCH 15/18] run flink test on label --- .github/workflows/dataflow.yaml | 6 ++++-- .github/workflows/flink.yaml | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/workflows/dataflow.yaml b/.github/workflows/dataflow.yaml index 12195f23..28d321d7 100644 --- a/.github/workflows/dataflow.yaml +++ b/.github/workflows/dataflow.yaml @@ -14,12 +14,14 @@ jobs: # run on: # - all pushes to main # - schedule defined above - # - a PR was just labeled 'test-dataflow' - # - a PR with 'test-dataflow' label was opened, reopened, or synchronized + # - a PR was just labeled 'test-dataflow' or 'test-all' + # - a PR with 'test-dataflow' or 'test-all' label was opened, reopened, or synchronized if: | github.event_name == 'push' || github.event_name == 'schedule' || + github.event.label.name == 'test-all' || github.event.label.name == 'test-dataflow' || + contains( github.event.pull_request.labels.*.name, 'test-all') || contains( github.event.pull_request.labels.*.name, 'test-dataflow') runs-on: ubuntu-latest strategy: diff --git a/.github/workflows/flink.yaml b/.github/workflows/flink.yaml index cd71dccb..eb6655f8 100644 --- a/.github/workflows/flink.yaml +++ b/.github/workflows/flink.yaml @@ -5,9 +5,24 @@ on: branches: [ "main" ] pull_request: branches: [ "main" ] + types: [ opened, reopened, synchronize, labeled ] + schedule: + - cron: '0 4 * * *' # run once a day at 4 AM jobs: build: + # run on: + # - all pushes to main + # - schedule defined above + # - a PR was just labeled 'test-flink' or 'test-all' + # - a PR with 'test-flink' or 'test-all' label was opened, reopened, or synchronized + if: | + github.event_name == 'push' || + github.event_name == 'schedule' || + github.event.label.name == 'test-all' || + github.event.label.name == 'test-flink' || + contains( github.event.pull_request.labels.*.name, 'test-all') || + contains( github.event.pull_request.labels.*.name, 'test-flink') runs-on: ubuntu-latest From 502735b31f2661b5fb60fca6a36bed58cc38d4ce Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Fri, 1 Sep 2023 11:49:24 -0700 Subject: [PATCH 16/18] fix conditional in config file error catcher --- pangeo_forge_runner/commands/base.py | 2 +- tests/unit/test_expand_meta.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pangeo_forge_runner/commands/base.py b/pangeo_forge_runner/commands/base.py index 5201dbca..6ed97607 100644 --- a/pangeo_forge_runner/commands/base.py +++ b/pangeo_forge_runner/commands/base.py @@ -213,7 +213,7 @@ def initialize(self, argv=None): super().initialize(argv) # Load traitlets config from a config file if passed if self.config_file: - if os.path.exists(self.config_file): + if not os.path.exists(self.config_file): # Throw an explicit error and exit if config file isn't present print( f"Could not read config from file {self.config_file}. Make sure it exists and is readable", diff --git a/tests/unit/test_expand_meta.py b/tests/unit/test_expand_meta.py index e80b3cc5..4beb35bf 100644 --- a/tests/unit/test_expand_meta.py +++ b/tests/unit/test_expand_meta.py @@ -83,5 +83,6 @@ def test_missing_config_file(): non_existent_path = secrets.token_hex() + ".py" assert not os.path.exists(non_existent_path) cmd = ["pangeo-forge-runner", "expand-meta", "--config", non_existent_path] - proc = subprocess.run(cmd, encoding="utf-8") + proc = subprocess.run(cmd, encoding="utf-8", capture_output=True, text=True) assert proc.returncode == 1 + assert "Could not read config from file" in proc.stderr From 1a51ec1a49452426158590d8301475a1d1b02ac5 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Fri, 1 Sep 2023 12:30:09 -0700 Subject: [PATCH 17/18] don't throw error if default config is missing --- pangeo_forge_runner/commands/base.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pangeo_forge_runner/commands/base.py b/pangeo_forge_runner/commands/base.py index 6ed97607..ed807b00 100644 --- a/pangeo_forge_runner/commands/base.py +++ b/pangeo_forge_runner/commands/base.py @@ -213,7 +213,11 @@ def initialize(self, argv=None): super().initialize(argv) # Load traitlets config from a config file if passed if self.config_file: - if not os.path.exists(self.config_file): + self.load_config_file(self.config_file) + if ( + not os.path.exists(self.config_file) + and not self.config_file == "pangeo_forge_runner_config.py" + ): # Throw an explicit error and exit if config file isn't present print( f"Could not read config from file {self.config_file}. Make sure it exists and is readable", @@ -221,8 +225,6 @@ def initialize(self, argv=None): ) sys.exit(1) - self.load_config_file(self.config_file) - # Allow arbitrary logging config if set # We do this first up so any custom logging we set up ourselves # is not affected, as by default dictConfig will replace all From fd77b7d617d6ec1546baa0ffeafda9ed90f0dc81 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Fri, 1 Sep 2023 12:40:45 -0700 Subject: [PATCH 18/18] use != instead of not == --- pangeo_forge_runner/commands/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pangeo_forge_runner/commands/base.py b/pangeo_forge_runner/commands/base.py index ed807b00..b235d7fb 100644 --- a/pangeo_forge_runner/commands/base.py +++ b/pangeo_forge_runner/commands/base.py @@ -216,7 +216,7 @@ def initialize(self, argv=None): self.load_config_file(self.config_file) if ( not os.path.exists(self.config_file) - and not self.config_file == "pangeo_forge_runner_config.py" + and self.config_file != "pangeo_forge_runner_config.py" ): # Throw an explicit error and exit if config file isn't present print(