Skip to content

Commit

Permalink
Feat!: remove default values from TaskDef and WorkflowDef (#452)
Browse files Browse the repository at this point in the history
# The problem
Using default values create problems when we want to copy those objects
around. One might forget parameter.
https://zapatacomputing.atlassian.net/browse/ORQSDK-1054

# This PR's solution
Remove default parameters for WorkflowDef and TaskDef objects

Note:
This is marked as breaking change, as those objects are within public
API - although they were not supposed to be created by users so we
should not expect any incompatibilities

# Checklist

_Check that this PR satisfies the following items:_

- [x] Tests have been added for new features/changed behavior (if no new
features have been added, check the box).
- [x] The [changelog file](CHANGELOG.md) has been updated with a
user-readable description of the changes (if the change isn't visible to
the user in any way, check the box).
- [x] The PR's title is prefixed with
`<feat/fix/chore/imp[rovement]/int[ernal]/docs>[!]:`
- [x] The PR is linked to a JIRA ticket (if there's no suitable ticket,
check the box).
  • Loading branch information
SebastianMorawiec authored Oct 9, 2024
1 parent d33b552 commit 404ec9b
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 19 deletions.
1 change: 1 addition & 0 deletions projects/orquestra-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
🚨 *Breaking Changes*

* Setting `workspace_id` is now required when submitting workflow to remote clusters
* `TaskDef` `WorkflowDef` and `TaskInvocation` Objects `__init__` functions now do set parameters by default.

🔥 *Features*

Expand Down
31 changes: 17 additions & 14 deletions projects/orquestra-sdk/src/orquestra/sdk/_client/_base/_dsl.py
Original file line number Diff line number Diff line change
Expand Up @@ -538,15 +538,15 @@ def __init__(
self,
fn: Callable[..., _TaskReturn],
output_metadata: "orquestra.sdk._client._base._dsl.TaskOutputMetadata", # pyright: ignore # NOQA
source_import: Optional[Import] = None,
parameters: Optional[OrderedDict] = None,
dependency_imports: Optional[Tuple[Import, ...]] = None,
resources: Resources = Resources(),
custom_image: Optional[str] = None,
custom_name: Optional[str] = None,
fn_ref: Optional[FunctionRef] = None,
max_retries: Optional[int] = None,
env_vars: Optional[Dict[str, str]] = None,
source_import: Optional[Import],
parameters: Optional[OrderedDict],
dependency_imports: Optional[Tuple[Import, ...]],
resources: Resources,
custom_image: Optional[str],
custom_name: Optional[str],
fn_ref: Optional[FunctionRef],
max_retries: Optional[int],
env_vars: Optional[Dict[str, str]],
):
if isinstance(fn, BuiltinFunctionType):
raise NotImplementedError("Built-in functions are not supported as Tasks")
Expand Down Expand Up @@ -670,6 +670,7 @@ def __call__(
custom_name=parse_custom_name(self._custom_name, signature),
custom_image=self._custom_image,
env_vars=self._env_vars,
type="task_invocation",
)
)

Expand Down Expand Up @@ -731,11 +732,11 @@ def __init__(
task: TaskDef[_TaskReturn],
args: Tuple[Argument, ...],
kwargs: Tuple[Tuple[str, Argument], ...],
type: str = "task_invocation",
resources: Resources = Resources(),
custom_name: Optional[str] = None,
custom_image: Optional[str] = None,
env_vars: Optional[Dict[str, str]] = None,
type: str,
resources: Resources,
custom_name: Optional[str],
custom_image: Optional[str],
env_vars: Optional[Dict[str, str]],
):
self.task = task
self.args = args
Expand Down Expand Up @@ -939,6 +940,7 @@ def with_invocation_meta(
custom_name=invocation.custom_name,
custom_image=new_custom_image,
env_vars=new_env_vars,
type=invocation.type,
)

return ArtifactFuture(
Expand Down Expand Up @@ -1296,6 +1298,7 @@ def _inner(fn: Callable):
custom_name=custom_name,
max_retries=max_retries,
env_vars=env_vars,
fn_ref=None,
)

return task_def
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ def __init__(
fn_ref: FunctionRef,
resources: _dsl.Resources,
head_node_image: Optional[str],
data_aggregation: Optional[DataAggregation] = None,
workflow_args: Optional[Tuple[Any, ...]] = None,
workflow_kwargs: Optional[Dict[str, Any]] = None,
default_source_import: Optional[Import] = None,
default_dependency_imports: Optional[Iterable[Import]] = None,
data_aggregation: Optional[DataAggregation],
workflow_args: Optional[Tuple[Any, ...]],
workflow_kwargs: Optional[Dict[str, Any]],
default_source_import: Optional[Import],
default_dependency_imports: Optional[Iterable[Import]],
):
self._name = name
self._fn = workflow_fn
Expand Down

0 comments on commit 404ec9b

Please sign in to comment.