Skip to content

add short_description to Task and Workflow model #3224

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add short_description to Task and Workflow model #3224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 15 additions & 2 deletions flytekit/models/admin/workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,14 @@


class Workflow(_common.FlyteIdlEntity):
def __init__(self, id, closure):
def __init__(self, id, closure, short_description):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to be short_description = None for backwards compat?

"""
:param flytekit.models.core.identifier.Identifier id:
:param WorkflowClosure closure:
"""
self._id = id
self._closure = closure
self._short_description = short_description

@property
def id(self):
Expand All @@ -92,11 +93,22 @@
"""
return self._closure

@property
def short_description(self):
"""
:rtype: str
"""
return self._short_description

Check warning on line 101 in flytekit/models/admin/workflow.py

View check run for this annotation

Codecov / codecov/patch

flytekit/models/admin/workflow.py#L101 

Added line #L101 was not covered by tests

def to_flyte_idl(self):
"""
:rtype: flyteidl.admin.workflow_pb2.Workflow
"""
return _admin_workflow.Workflow(id=self.id.to_flyte_idl(), closure=self.closure.to_flyte_idl())
return _admin_workflow.Workflow(

Check warning on line 107 in flytekit/models/admin/workflow.py

View check run for this annotation

Codecov / codecov/patch

flytekit/models/admin/workflow.py#L107 

Added line #L107 was not covered by tests
id=self.id.to_flyte_idl(),
closure=self.closure.to_flyte_idl(),
short_description=self.short_description,
)

@classmethod
def from_flyte_idl(cls, pb2_object):
Expand All @@ -107,6 +119,7 @@
return cls(
id=_identifier.Identifier.from_flyte_idl(pb2_object.id),
closure=WorkflowClosure.from_flyte_idl(pb2_object.closure),
short_description=pb2_object.short_description,
)


Expand Down
12 changes: 11 additions & 1 deletion flytekit/models/task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,13 +686,14 @@


class Task(_common.FlyteIdlEntity):
def __init__(self, id, closure):
def __init__(self, id, closure, short_description):
"""
:param flytekit.models.core.identifier.Identifier id: The (project, domain, name) identifier for this task.
:param TaskClosure closure: The closure for the underlying workload.
"""
self._id = id
self._closure = closure
self._short_description = short_description

@property
def id(self):
Expand All @@ -710,13 +711,21 @@
"""
return self._closure

@property
def short_description(self):
"""
:rtype: str
"""
return self._short_description

Check warning on line 719 in flytekit/models/task.py

View check run for this annotation

Codecov / codecov/patch

flytekit/models/task.py#L719 

Added line #L719 was not covered by tests

def to_flyte_idl(self):
"""
:rtype: flyteidl.admin.task_pb2.Task
"""
return _admin_task.Task(
closure=self.closure.to_flyte_idl(),
id=self.id.to_flyte_idl(),
short_description=self.short_description,
)

@classmethod
Expand All @@ -728,6 +737,7 @@
return cls(
closure=TaskClosure.from_flyte_idl(pb2_object.closure),
id=_identifier.Identifier.from_flyte_idl(pb2_object.id),
short_description=pb2_object.short_description,
)


Expand Down
11 changes: 8 additions & 3 deletions tests/flytekit/unit/remote/test_remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,12 +380,15 @@ def get_compiled_workflow_closure():
def test_fetch_lazy(remote):
mock_client = remote._client
mock_client.get_task.return_value = Task(
id=Identifier(ResourceType.TASK, "p", "d", "n", "v"), closure=LIST_OF_TASK_CLOSURES[0]
id=Identifier(ResourceType.TASK, "p", "d", "n", "v"),
closure=LIST_OF_TASK_CLOSURES[0],
short_description="task description",
)

mock_client.get_workflow.return_value = Workflow(
id=Identifier(ResourceType.TASK, "p", "d", "n", "v"),
closure=WorkflowClosure(compiled_workflow=get_compiled_workflow_closure()),
short_description="workflow description",
)

lw = remote.fetch_workflow_lazy(name="wn", version="v")
Expand Down Expand Up @@ -443,8 +446,9 @@ def test_launch_backfill(remote):
mock_client.get_workflow.return_value = Workflow(
id=Identifier(ResourceType.WORKFLOW, "p", "d", "daily2", "v"),
closure=WorkflowClosure(
compiled_workflow=CompiledWorkflowClosure(primary=ser_wf, sub_workflows=[], tasks=tasks)
compiled_workflow=CompiledWorkflowClosure(primary=ser_wf, sub_workflows=[], tasks=tasks),
),
short_description="workflow description",
)

wf = remote.launch_backfill(
Expand All @@ -468,6 +472,7 @@ def test_fetch_workflow_with_branch(mock_promote, mock_workflow, remote):
mock_client.get_workflow.return_value = Workflow(
id=Identifier(ResourceType.TASK, "p", "d", "n", "v"),
closure=WorkflowClosure(compiled_workflow=MagicMock()),
short_description="workflow description",
)

admin_launch_plan = MagicMock()
Expand All @@ -486,6 +491,7 @@ def test_fetch_workflow_with_nested_branch(mock_promote, mock_workflow, remote):
mock_client.get_workflow.return_value = Workflow(
id=Identifier(ResourceType.TASK, "p", "d", "n", "v"),
closure=WorkflowClosure(compiled_workflow=MagicMock()),
short_description="workflow description",
)
admin_launch_plan = MagicMock()
admin_launch_plan.spec = {"workflow_id": 123}
Expand Down Expand Up @@ -856,7 +862,6 @@ def workflow1():
assert isinstance(registered_workflow, FlyteWorkflow)
assert registered_workflow.id == Identifier(ResourceType.WORKFLOW, "flytesnacks", "development", "tests.flytekit.unit.remote.test_remote.workflow1", "dummy_version")


@mock.patch("flytekit.remote.remote.get_serializable")
@mock.patch("flytekit.remote.remote.FlyteRemote.fetch_launch_plan")
@mock.patch("flytekit.remote.remote.FlyteRemote.raw_register")
Expand Down
Loading