Skip to content

Commit

Permalink
Fix global config issue with script command (#542)
Browse files Browse the repository at this point in the history
This should the last of such bugs. Went through the codebase and ensured
we were not using global config elsewhere like this.

Signed-off-by: Sambhav Kothari <[email protected]>
  • Loading branch information
sambhav authored Apr 1, 2023
1 parent f2bf6e2 commit fe6ef10
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 17 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ lint: ## Run a `lint` process on Hera and report problems

.PHONY: test
test: ## Run tests for Hera
@poetry run python -m pytest -v
@poetry run python -m pytest

.PHONY: workflows-models
workflows-models: ## Generate the Workflows models portion of Argo Workflows
Expand Down
16 changes: 11 additions & 5 deletions docs/examples/workflows/global_config.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
global_config.namespace = "argo-namespace"
global_config.service_account_name = "argo-account"
global_config.image = "image-say"
global_config.script_command = ["python3"]


@script()
Expand Down Expand Up @@ -43,14 +44,19 @@
command:
- cowsay
image: docker/whalesay:latest
- name: 'say'
- name: say
script:
command: ['python']
image: 'image-say'
source: |
import os
command:
- python3
image: image-say
source: 'import os

import sys

sys.path.append(os.getcwd())

print("hello")

'
```

15 changes: 10 additions & 5 deletions examples/workflows/global-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@ spec:
command:
- cowsay
image: docker/whalesay:latest
- name: 'say'
- name: say
script:
command: ['python']
image: 'image-say'
source: |
import os
command:
- python3
image: image-say
source: 'import os
import sys
sys.path.append(os.getcwd())
print("hello")
'
1 change: 1 addition & 0 deletions examples/workflows/global_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
global_config.namespace = "argo-namespace"
global_config.service_account_name = "argo-account"
global_config.image = "image-say"
global_config.script_command = ["python3"]


@script()
Expand Down
136 changes: 134 additions & 2 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ types-PyYAML = "*"
jsonpath-ng = "^1.5.3"
datamodel-code-generator = {extras = ["http"], version = "^0.17.1"}
types-requests = "^2.28.11.12"
pytest-clarity = "^1.0.1"
pytest-sugar = "^0.9.6"

[build-system]
requires = ["poetry-core>=1.0.0"]
Expand All @@ -59,7 +61,7 @@ max-line-length = 119
target-version = ['py38']

[tool.pytest.ini_options]
addopts = "--durations=5 -vv --cov=hera --cov-report=term-missing --cov-report=xml --cov-config=pyproject.toml"
addopts = "-vv --cov=hera --cov-report=xml --cov-config=pyproject.toml"
filterwarnings = [
# Hide the hera.host_config deprecations
'ignore:.*is deprecated in favor of `global_config.GlobalConfig',
Expand Down
6 changes: 4 additions & 2 deletions src/hera/shared/_global_config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import inspect
from dataclasses import dataclass, field
from typing import Callable, Dict, List, Optional, Type, Union

from ._base_model import TBase
Expand All @@ -9,6 +10,7 @@
_HookMap = Dict[Type[TBase], List[Hook]]


@dataclass
class _GlobalConfig:
"""Hera global configuration holds any user configuration such as global tokens, hooks, etc.
Expand All @@ -27,12 +29,12 @@ class _GlobalConfig:
namespace: Optional[str] = None
_image: Union[str, Callable[[], str]] = "python:3.7"
service_account_name: Optional[str] = None
script_command: Optional[List[str]] = ["python"]
script_command: Optional[List[str]] = field(default_factory=lambda: ["python"])
_pre_build_hooks: Optional[_HookMap] = None

def reset(self) -> None:
"""Resets the global config container to its initial state"""
self.__dict__.clear() # Wipe instance values to fallback to the class defaults
self.__dict__ = _GlobalConfig().__dict__

@property
def image(self) -> str:
Expand Down
11 changes: 10 additions & 1 deletion src/hera/workflows/script.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import textwrap
from typing import Callable, Dict, List, Optional, Union

from pydantic import validator

from hera.shared import global_config
from hera.workflows._mixins import (
CallableTemplateMixin,
Expand Down Expand Up @@ -43,13 +45,20 @@ class Script(

container_name: Optional[str] = None
args: Optional[List[str]] = None
command: Optional[List[str]] = global_config.script_command
command: Optional[List[str]] = None
lifecycle: Optional[Lifecycle] = None
security_context: Optional[SecurityContext] = None
source: Optional[Union[Callable, str]] = None
working_dir: Optional[str] = None
add_cwd_to_sys_path: bool = True

@validator("command", pre=True, always=True)
@classmethod
def _check_command(cls, v):
if v is None:
return global_config.script_command
return v

def _get_param_script_portion(self) -> str:
"""Constructs and returns a script that loads the parameters of the specified arguments. Since Argo passes
parameters through {{input.parameters.name}} it can be very cumbersome for users to manage that. This creates a
Expand Down

0 comments on commit fe6ef10

Please sign in to comment.