Skip to content

Commit

Permalink
feat!: Update openjd-cli to pass template_dir/cwd to preprocess_job_p…
Browse files Browse the repository at this point in the history
…arameters (#29)

* Update the openjd-sessions dependency to 0.3.*
* Modify the use of preprocess_job_parameters, and the tests, to account
  for the upstream change
* Update to reflect type changes from openjd-sessions
* Add Python 3.12 and Windows to the gihub actions configuration.

Signed-off-by: Mark Wiebe <399551+mwiebe@users.noreply.github.com>
  • Loading branch information
mwiebe authored Jan 19, 2024
1 parent bbd61d0 commit 0983e1d
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 51 deletions.
19 changes: 13 additions & 6 deletions .github/workflows/reuse_python_build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ jobs:
contents: read
strategy:
matrix:
python-version: ['3.9', '3.10', '3.11']
os: [ubuntu-latest,macOS-latest]
python-version: ['3.9', '3.10', '3.11', '3.12']
os: [ubuntu-latest, windows-latest, macOS-latest]
env:
PYTHON: ${{ matrix.python-version }}
CODEARTIFACT_REGION: "us-west-2"
Expand All @@ -29,7 +29,7 @@ jobs:
steps:
- uses: actions/checkout@v4
if: ${{ !inputs.branch }}

- uses: actions/checkout@v4
if: ${{ inputs.branch }}
with:
Expand All @@ -40,26 +40,33 @@ jobs:
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}

- name: Configure AWS credentials
uses: aws-actions/configure-aws-credentials@v4
with:
role-to-assume: ${{ secrets.AWS_CODEARTIFACT_ROLE }}
aws-region: us-west-2
mask-aws-account-id: true

- name: CodeArtifact Setup Windows
if: ${{ matrix.os == 'windows-latest' }}
run: |
$CODEARTIFACT_AUTH_TOKEN=$(aws codeartifact get-authorization-token --domain ${{ secrets.CODEARTIFACT_DOMAIN }} --domain-owner ${{ secrets.CODEARTIFACT_ACCOUNT_ID }} --query authorizationToken --output text --region us-west-2)
echo "::add-mask::$CODEARTIFACT_AUTH_TOKEN"
echo CODEARTIFACT_AUTH_TOKEN=$CODEARTIFACT_AUTH_TOKEN >> $env:GITHUB_ENV
- name: CodeArtifact Setup Linux/MacOS
if: ${{ matrix.os != 'windows-latest' }}
run: |
CODEARTIFACT_AUTH_TOKEN=$(aws codeartifact get-authorization-token --domain ${{ secrets.CODEARTIFACT_DOMAIN }} --domain-owner ${{ secrets.CODEARTIFACT_ACCOUNT_ID }} --query authorizationToken --output text --region us-west-2)
echo "::add-mask::$CODEARTIFACT_AUTH_TOKEN"
echo CODEARTIFACT_AUTH_TOKEN=$CODEARTIFACT_AUTH_TOKEN >> $GITHUB_ENV
- name: Install Dependencies
run: pip install --upgrade -r requirements-development.txt

- name: Run Linting
run: hatch run lint

- name: Run Tests
run: hatch run test
run: hatch run test
6 changes: 3 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ license = "Apache-2.0"
requires-python = ">=3.9"

dependencies = [
"openjd-sessions == 0.2.*"
"openjd-sessions == 0.3.*"
]

[project.scripts]
Expand Down Expand Up @@ -125,7 +125,7 @@ addopts = [
"--cov-report=xml:build/coverage/coverage.xml",
"--cov-report=term-missing",
"--numprocesses=auto",
"--timeout=30"
"--timeout=60"
]


Expand All @@ -141,4 +141,4 @@ source = [

[tool.coverage.report]
show_missing = true
fail_under = 92
fail_under = 92
11 changes: 8 additions & 3 deletions src/openjd/cli/_common/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from typing import Callable, Literal
import json
import yaml
import os

from ._job_from_template import (
job_from_template,
Expand Down Expand Up @@ -100,14 +101,18 @@ def add(self, name: str, description: str, **kwargs) -> ArgumentParser:
def generate_job(args: Namespace) -> Job:
try:
# Raises: RuntimeError, DecodeValidationError
_, template = read_template(args)
template_file, template = read_template(args)
# Raises: RuntimeError
sample_job = job_from_template(template, args.job_params if args.job_params else None)
return job_from_template(
template,
args.job_params if args.job_params else None,
Path(os.path.abspath(template_file.parent)),
Path(os.getcwd()),
)
except RuntimeError as rte:
raise RuntimeError(f"ERROR generating Job: {str(rte)}")
except DecodeValidationError as dve:
raise RuntimeError(f"ERROR validating template: {str(dve)}")
return sample_job


@dataclass
Expand Down
12 changes: 10 additions & 2 deletions src/openjd/cli/_common/_job_from_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,12 @@ def get_task_params(arguments: list[list[str]]) -> list[dict[str, str]]:
return all_parameter_sets


def job_from_template(template: JobTemplate, parameter_args: list[str] | None = None) -> Job:
def job_from_template(
template: JobTemplate,
parameter_args: list[str] | None,
job_template_dir: Path,
current_working_dir: Path,
) -> Job:
"""
Given a decoded Job Template and a user-inputted parameter dictionary,
generates a Job object.
Expand All @@ -170,7 +175,10 @@ def job_from_template(template: JobTemplate, parameter_args: list[str] | None =

try:
parameter_values = preprocess_job_parameters(
job_template=template, job_parameter_values=parameter_dict
job_template=template,
job_parameter_values=parameter_dict,
job_template_dir=job_template_dir,
current_working_dir=current_working_dir,
)
except ValueError as ve:
raise RuntimeError(f"Parameters can't be used with Template: {str(ve)}")
Expand Down
13 changes: 6 additions & 7 deletions src/openjd/cli/_run/_local_session/_session_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
ActionState,
ActionStatus,
Parameter,
ParameterType,
Session,
SessionState,
PathMappingRule,
Expand Down Expand Up @@ -70,7 +69,7 @@ def __init__(
# Evaluate Job parameters, if applicable
if job.parameters:
parameters_as_list = [
Parameter(type=ParameterType(param.type.name), name=name, value=param.value)
Parameter(type=ParameterValueType(param.type.name), name=name, value=param.value)
for (name, param) in job.parameters.items()
]

Expand Down Expand Up @@ -170,7 +169,7 @@ def initialize(
step=dep,
parameters=[
Parameter(
type=ParameterType(param.type.name),
type=param.type,
name=name,
value=param.value,
)
Expand All @@ -185,9 +184,9 @@ def initialize(

else:
if not task_parameter_values:
parameter_sets = [
param_set for param_set in StepParameterSpaceIterator(space=step.parameterSpace)
]
parameter_sets: list[TaskParameterSet] = list(
StepParameterSpaceIterator(space=step.parameterSpace)
)
else:
try:
parameter_sets = [
Expand Down Expand Up @@ -217,7 +216,7 @@ def initialize(
step=step,
parameters=[
Parameter(
type=ParameterType(param.type.name),
type=param.type,
name=name,
value=param.value,
)
Expand Down
31 changes: 26 additions & 5 deletions test/openjd/cli/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

import os
import pytest
import tempfile
from pathlib import Path
from unittest.mock import patch

from . import MOCK_TEMPLATE, SampleSteps
Expand All @@ -10,18 +13,36 @@


@pytest.fixture(scope="function", params=[[], ["Message='A new message!'"]])
def sample_job(request):
def sample_job_and_dirs(request):
"""
Uses the MOCK_TEMPLATE object to create a Job, once
with default parameters and once with user-specified parameters.
This fixture also manages the life time of a temporary directory that's
used for the job template dir and the current working directory.
"""
template = decode_template(template=MOCK_TEMPLATE)
return job_from_template(template=template, parameter_args=request.param)
with tempfile.TemporaryDirectory() as tmpdir:
template_dir = Path(tmpdir) / "template_dir"
current_working_dir = Path(tmpdir) / "current_working_dir"
os.makedirs(template_dir)
os.makedirs(current_working_dir)

template = decode_template(template=MOCK_TEMPLATE)
yield (
job_from_template(
template=template,
parameter_args=request.param,
job_template_dir=template_dir,
current_working_dir=current_working_dir,
),
template_dir,
current_working_dir,
)


@pytest.fixture(scope="function")
def sample_step_map(sample_job):
return {step.name: step for step in sample_job.steps}
def sample_step_map(sample_job_and_dirs):
return {step.name: step for step in sample_job_and_dirs[0].steps}


@pytest.fixture(
Expand Down
33 changes: 28 additions & 5 deletions test/openjd/cli/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@
)


@pytest.fixture(scope="function")
def template_dir_and_cwd():
"""
This fixture manages the life time of a temporary directory that's
used for the job template dir and the current working directory.
"""
with tempfile.TemporaryDirectory() as tmpdir:
template_dir = Path(tmpdir) / "template_dir"
current_working_dir = Path(tmpdir) / "current_working_dir"
os.makedirs(template_dir)
os.makedirs(current_working_dir)

yield (template_dir, current_working_dir)


@patch("openjd.cli._common._validation_utils.decode_template")
def test_decode_template_called(mock_decode_template: Mock):
"""
Expand Down Expand Up @@ -335,13 +350,16 @@ def test_get_job_params_error(
),
],
)
def test_job_from_template_success(mock_params: list, expected_job_name: str, template_dict: dict):
def test_job_from_template_success(
mock_params: list, expected_job_name: str, template_dict: dict, template_dir_and_cwd: tuple
):
"""
Test that `job_from_template` creates a Job with the provided parameters.
"""
template_dir, current_working_dir = template_dir_and_cwd
template = decode_template(template=template_dict)

result = job_from_template(template, mock_params)
result = job_from_template(template, mock_params, template_dir, current_working_dir)
assert result.name == expected_job_name
assert result.steps == template.steps
if result.parameters:
Expand Down Expand Up @@ -377,15 +395,18 @@ def test_job_from_template_success(mock_params: list, expected_job_name: str, te
),
],
)
def test_job_from_template_error(mock_params: list, template_dict: dict, expected_error: str):
def test_job_from_template_error(
mock_params: list, template_dict: dict, expected_error: str, template_dir_and_cwd: tuple
):
"""
Test that errors thrown by `job_from_template` have expected information
"""
template_dir, current_working_dir = template_dir_and_cwd

template = decode_template(template=template_dict)

with pytest.raises(RuntimeError) as rte:
job_from_template(template, mock_params)
job_from_template(template, mock_params, template_dir, current_working_dir)

assert expected_error in str(rte.value)

Expand Down Expand Up @@ -563,6 +584,8 @@ def test_generate_job_success(
new=Mock(side_effect=job_from_template),
) as patched_job_from_template:
generate_job(mock_args)
patched_job_from_template.assert_called_once_with(ANY, expected_param_list)
patched_job_from_template.assert_called_once_with(
ANY, expected_param_list, Path(temp_template.name).parent, Path(os.getcwd())
)

Path(temp_template.name).unlink()
Loading

0 comments on commit 0983e1d

Please sign in to comment.