Skip to content

Commit

Permalink
Merge pull request #13 from pangeo-forge/assorted-fixes
Browse files Browse the repository at this point in the history
Assorted fixes & enhancements
  • Loading branch information
cisaacstern authored Aug 1, 2023
2 parents 3fdd9aa + 54dc84b commit 256da29
Show file tree
Hide file tree
Showing 5 changed files with 233 additions and 30 deletions.
35 changes: 35 additions & 0 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Test

on:
push:
branches: [ "main" ]
pull_request:
branches: [ "*" ]

jobs:
test:

runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ["3.9.13"] # consistent with Dockerfile

steps:
- uses: actions/checkout@v3
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v3
with:
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install -r dev-requirements.txt
- name: Test with pytest
run: |
pytest -vvv -s --cov=action action/test_main.py
- name: Upload Coverage to Codecov
uses: codecov/codecov-action@v2
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,6 @@ cython_debug/
# and can be added to the global gitignore or merged into this file. For a more nuclear
# option (not recommended) you can uncomment the following to ignore the entire idea folder.
#.idea/

# vscode
.vscode
67 changes: 37 additions & 30 deletions action/deploy_recipe.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import json
import os
import subprocess
import sys
import tempfile
from urllib.parse import urljoin

import requests

Expand All @@ -26,15 +26,15 @@ def deploy_recipe_cmd(cmd: list[str]):
print(f"Job submitted with {job_id = } and {job_name = }")


if __name__ == "__main__":
def main():
# set in Dockerfile
conda_env = os.environ["CONDA_ENV"]

# injected by github actions
repository = os.environ["GITHUB_REPOSITORY"] # will this fail for external prs?
server_url = os.environ["GITHUB_SERVER_URL"]
api_url = os.environ["GITHUB_API_URL"]
ref = os.environ["GITHUB_HEAD_REF"]
head_ref = os.environ["GITHUB_HEAD_REF"]
sha = os.environ["GITHUB_SHA"]
repository_id = os.environ["GITHUB_REPOSITORY_ID"]
run_id = os.environ["GITHUB_RUN_ID"]
run_attempt = os.environ["GITHUB_RUN_ATTEMPT"]
Expand All @@ -43,42 +43,49 @@ def deploy_recipe_cmd(cmd: list[str]):
config = json.loads(os.environ["INPUT_PANGEO_FORGE_RUNNER_CONFIG"])
select_recipe_by_label = os.environ["INPUT_SELECT_RECIPE_BY_LABEL"]

# assemble https url for pangeo-forge-runner
repo = urljoin(server_url, repository)

# log variables to stdout
print(f"{conda_env = }")
print(f"{repo = }")
print(f"{ref = }")
print(f"{head_ref = }")
print(f"{sha = }")
print(f"{config = }")

labels = []
if select_recipe_by_label:
# iterate through PRs on the repo. if the PR's head ref is the same
# as our ``ref`` here, get the labels from that PR, and stop iteration.
# FIXME: what if this is a push event, and not a pull_request event?
pulls_url = "/".join([api_url, "repos", repository, "pulls"])
print(f"Fetching pulls from {pulls_url}")
# `head_ref` is available for `pull_request` events. on a `push` event, the `head_ref`
# environment variable will be empty, so in that case we use `sha` to find the PR instead.
commit_sha = head_ref if head_ref else sha
# querying the following endpoint should give us the PR containing the desired labels,
# regardless of whether this is a `pull_request` or `push` event. see official docs here:
# https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-pull-requests-associated-with-a-commit
pulls_url = "/".join([api_url, "repos", repository, "commits", commit_sha, "pulls"])
headers = {"X-GitHub-Api-Version": "2022-11-28", "Accept": "application/vnd.github+json"}

pulls_response = requests.get(pulls_url)
print(f"Fetching pulls from {pulls_url}")
pulls_response = requests.get(pulls_url, headers=headers)
pulls_response.raise_for_status()
pulls = pulls_response.json()
assert len(pulls) == 1 # pretty sure this is always true, but just making sure
labels: list[str] = [label["name"] for label in pulls[0]["labels"]]

for p in pulls:
if p["head"]["ref"] == ref:
labels: list[str] = [label["name"] for label in p["labels"]]
recipe_ids = [l.replace("run:", "") for l in labels if l.startswith("run:")]

# dynamically install extra deps if requested.
# if calling `pangeo-forge-runner` directly, `--feedstock-subdir` can be passed as a CLI arg.
# in the action context, users do not compose their own `pangeo-forge-runner` CLI calls, so if
# they want to use a non-default value for feedstock-subdir, it must be passed via the long-form
# name in the config JSON (i.e, `{"BaseCommand": "feedstock-subdir": ...}}`).
feedstock_subdir = (
config["BaseCommand"]["feedstock-subdir"]
if "BaseCommand" in config and "feedstock-subdir" in config["BaseCommand"]
else "feedstock"
)
# because we've run the actions/checkout step before reaching this point, our current
# working directory is the root of the feedstock repo, so we can list feedstock repo
# contents directly on the filesystem here, without requesting it from github.
if "requirements.txt" in os.listdir("feedstock"):
with open("feedstock/requirements.txt") as f:
to_install = f.read().splitlines()

print(f"Installing extra packages {to_install}...")
install_cmd = f"mamba run -n {conda_env} pip install -U".split() + to_install
if "requirements.txt" in os.listdir(feedstock_subdir):
to_install = f"{feedstock_subdir}/requirements.txt"
print(f"Installing extra packages from {to_install}...")
install_cmd = f"mamba run -n {conda_env} pip install -Ur {to_install}".split()
install_proc = subprocess.run(install_cmd, capture_output=True, text=True)
if install_proc.returncode != 0:
# installations failed, so record the error and bail early
Expand All @@ -90,13 +97,9 @@ def deploy_recipe_cmd(cmd: list[str]):
cmd = [
"pangeo-forge-runner",
"bake",
"--repo",
repo,
"--ref",
ref,
"--repo=.",
"--json",
"-f",
f.name,
f"-f={f.name}",
]
print("\nSubmitting job...")
print(f"{recipe_ids = }")
Expand All @@ -118,3 +121,7 @@ def deploy_recipe_cmd(cmd: list[str]):
# passing a `--Bake.job_name_append` option to pangeo-forge-runner, which is a user-
# defined string to append to the job names.
deploy_recipe_cmd(cmd)


if __name__ == "__main__":
sys.exit(main())
155 changes: 155 additions & 0 deletions action/test_main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import os
from unittest.mock import MagicMock, patch

import pytest

from deploy_recipe import main


@pytest.fixture(params=["", "abcdefg"])
def head_ref(request):
return request.param


@pytest.fixture(params=["", "true"])
def select_recipe_by_label(request):
return request.param


@pytest.fixture
def env(select_recipe_by_label, head_ref):
return {
"CONDA_ENV": "notebook",
"GITHUB_REPOSITORY": "my/repo",
"GITHUB_API_URL": "https://api.github.com",
# fixturing of `head_ref` reflects that on `push`
# events, `GITHUB_HEAD_REF` env var is empty
"GITHUB_HEAD_REF": head_ref,
"GITHUB_SHA": "gfedcba",
"GITHUB_REPOSITORY_ID": "1234567890",
"GITHUB_RUN_ID": "0987654321",
"GITHUB_RUN_ATTEMPT": "1",
# TODO: parametrize runner config with `BaseCommand.feedstock-subdir`
"INPUT_PANGEO_FORGE_RUNNER_CONFIG": '{"a": "b"}',
"INPUT_SELECT_RECIPE_BY_LABEL": select_recipe_by_label,
}


@pytest.fixture
def requests_get_returns_json():
return [
{"labels": [{"name": "run:my-recipe"}]}
]


@pytest.fixture
def subprocess_return_values():
return dict(
stdout='{"job_id": "foo", "job_name": "bar"}',
stderr="",
returncode=0,
)


@pytest.fixture(
params=[
["meta.yaml", "recipe.py"],
["meta.yaml", "recipe.py", "requirements.txt"]
]
)
def listdir_return_value(request):
return request.param


@pytest.fixture
def mock_tempfile_name():
return "mock-temp-file.json"


@patch("deploy_recipe.os.listdir")
@patch("deploy_recipe.subprocess.run")
@patch("deploy_recipe.requests.get")
@patch("deploy_recipe.tempfile.NamedTemporaryFile")
def test_main(
# Note that patches are passed to `test_main` by order, not name.
# See https://docs.python.org/3/library/unittest.mock.html#quick-guide:
# "When you nest patch decorators the mocks are passed in to the decorated
# function in the same order they applied (the normal Python order that
# decorators are applied). This means from the bottom up..."
named_temporary_file: MagicMock,
requests_get: MagicMock,
subprocess_run: MagicMock,
listdir: MagicMock,
env: dict,
requests_get_returns_json: list,
subprocess_return_values: dict,
listdir_return_value: list,
mock_tempfile_name: str,
):
# mock a context manager, see: https://stackoverflow.com/a/28852060
named_temporary_file.return_value.__enter__.return_value.name = mock_tempfile_name

# mock reponse of requests.get call to github api
requests_get.return_value.json.return_value = requests_get_returns_json

# mock result of subprocess call to `pangeo-forge-runner`
subprocess_run.return_value.stdout.decode.return_value = subprocess_return_values["stdout"]
subprocess_run.return_value.stderr.decode.return_value = subprocess_return_values["stderr"]
subprocess_run.return_value.returncode = subprocess_return_values["returncode"]

# mock listdir call return value
listdir.return_value = listdir_return_value

with patch.dict(os.environ, env):
main()

if "requirements.txt" in listdir_return_value:
to_install = "feedstock/requirements.txt" # TODO: parametrize
subprocess_run.assert_any_call(
f"mamba run -n {env['CONDA_ENV']} pip install -Ur {to_install}".split(),
capture_output=True,
text=True,
)
else:
# if 'requirements.txt' not present, 'pip' is never invoked. re: `call_args_list`, see:
# 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 "pip" not in call

listdir.assert_called_once()

if env["INPUT_SELECT_RECIPE_BY_LABEL"]:
# requests.get is always called if INPUT_SELECT_RECIPE_BY_LABEL=true (to check github
# api for PR `run:...` labels). if this is a `pull_request` event, the called gh api
# url should contain the `GITHUB_HEAD_REF` for that PR. if this is a `push` event
# (reflected in the env by the fact that `GITHUB_HEAD_REF` is empty), then we expect to
# be calling an api url for the `GITHUB_SHA` associated with the `push`.
called_gh_api_url = requests_get.call_args[0][0]
if env["GITHUB_HEAD_REF"]:
assert env["GITHUB_HEAD_REF"] in called_gh_api_url
else:
assert env["GITHUB_SHA"] in called_gh_api_url

run_labels = [
label["name"].split("run:")[-1]
for pull_request in requests_get_returns_json
for label in pull_request["labels"]
if label["name"].startswith("run:")
]
if run_labels:
subprocess_run.assert_called_with(
[
'pangeo-forge-runner',
'bake',
'--repo=.',
'--json',
f'-f={mock_tempfile_name}',
f'--Bake.recipe_id={run_labels[-1]}',
f'--Bake.job_name=my-recipe-1234567890-0987654321-1'
],
capture_output=True,
)

else:
# subprocess.run is always called if INPUT_SELECT_RECIPE_BY_LABEL is empty
subprocess_run.assert_called()
3 changes: 3 additions & 0 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pytest
pytest-cov
requests

0 comments on commit 256da29

Please sign in to comment.