Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop explicit dependency on pangeo-forge-recipes #130

Merged
merged 5 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/flink.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ jobs:
python -m pip install --upgrade pip
python -m pip install -r dev-requirements.txt
python -m pip install -e .[flink]
python -m pip install -U ${{ matrix.recipes-version }}

- name: Set up min.io as a k3s service
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
run: |
python -m pip install --upgrade pip
python -m pip install -r dev-requirements.txt
python -m pip install -e .
python -m pip install -e ".[flink]"
python -m pip install -U ${{ matrix.recipes-version }}

- name: Test with pytest
Expand Down
5 changes: 5 additions & 0 deletions pangeo_forge_runner/commands/bake.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import re
import string
import time
from importlib.metadata import distributions
from pathlib import Path

import escapism
Expand Down Expand Up @@ -173,6 +174,10 @@ def start(self):
"""
Start the baking process
"""
if not "pangeo-forge-recipes" in [d.metadata["Name"] for d in distributions()]:
raise ValueError(
"To use the `bake` command, `pangeo-forge-recipes` must be installed."
)
# Create our storage configurations. Traitlets will do its magic, populate these
# with appropriate config from config file / commandline / defaults.
target_storage = TargetStorage(parent=self)
Expand Down
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
install_requires=[
"jupyter-repo2docker",
"ruamel.yaml",
"pangeo-forge-recipes>=0.9.2",
"escapism",
"jsonschema",
"traitlets",
Expand Down
5 changes: 5 additions & 0 deletions tests/integration/flink/test_flink_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import time
from importlib.metadata import version

import pytest
import xarray as xr
from packaging.version import parse as parse_version

Expand All @@ -20,6 +21,10 @@ def test_flink_bake(minio_service, flinkversion, pythonversion, beamversion):
recipe_version_ref = str(pfr_version)
else:
recipe_version_ref = "0.9.x"
pytest.xfail(
f"{pfr_version = }, which is < 0.10. "
"Flink tests timeout with this recipes version, so we xfail this test."
)

bucket = "s3://gpcp-out"
config = {
Expand Down
42 changes: 41 additions & 1 deletion tests/unit/test_bake.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import json
import re
import subprocess
import sys
import tempfile
from importlib.metadata import version
from importlib.metadata import distributions, version

import pytest
import xarray as xr
Expand All @@ -11,6 +12,45 @@
from pangeo_forge_runner.commands.bake import Bake


@pytest.fixture
def recipes_uninstalled():
"""Uninstall `pangeo-forge-recipes` for `test_bake_requires_recipes_installed`."""
# first confirm that it's installed to begin with
assert "pangeo-forge-recipes" in [d.metadata["Name"] for d in distributions()]
# and capture the version, which we'll reinstall after the test
recipes_version = parse_version(version("pangeo-forge-recipes"))
# now uninstall it
uninstall = subprocess.run(
f"{sys.executable} -m pip uninstall pangeo-forge-recipes -y".split()
)
assert uninstall.returncode == 0
assert "pangeo-forge-recipes" not in [d.metadata["Name"] for d in distributions()]
# and yield to the test
yield True
# test is complete, now reinstall pangeo-forge-recipes in the test env
reinstall = subprocess.run(
f"{sys.executable} -m pip install pangeo-forge-recipes=={recipes_version}".split()
)
assert reinstall.returncode == 0
# make sure it's there, and in the expected version
assert "pangeo-forge-recipes" in [d.metadata["Name"] for d in distributions()]
assert parse_version(version("pangeo-forge-recipes")) == recipes_version


def test_bake_requires_recipes_installed(recipes_uninstalled):
"""`pangeo-forge-runner` does not require `pangeo-forge-recipes` to be installed,
but `pangeo-forge-recipes` *is* required to use the `bake` command, so test that
we get a descriptive error if we try to invoke this command without it installed.
"""
assert recipes_uninstalled
bake = Bake()
with pytest.raises(
ValueError,
match="To use the `bake` command, `pangeo-forge-recipes` must be installed.",
):
bake.start()


@pytest.mark.parametrize(
"job_name, raises",
(
Expand Down