flytekit
flytekit copied to clipboard
Show version and execute by version
Tracking issue
fixes https://github.com/flyteorg/flyte/issues/4707
Why are the changes needed?
Show versions and creation time of remote-task/launchplan/workflow
What changes were proposed in this pull request?
This PR contains 2 parts.
For the show-versions part, I created a new top-level command VersionCommand
which inherits the RunCommand
class since they function similarly. Also, RemoteEntityVersionGroup
inherits RemoteEntityGroup
,and DynamicEntityVersionCommand
inherits both click.RichGroup
and DynamicEntityLaunchCommand
. It inherits click.RichGroup
because we want to display versions and creation time with click automatically.
We also extract parts of RunLevelParams
to create a base param class RunBaseParams
, so that VersionLevelParams
which inherits RunBaseParams
can display only the options that will be used is show-version command.
To fetch the creation time of the task, the created_at
attribute is added in closures. This ensures that both versions and created_time can be retrieved from list_tasks_paginated
method.
@classmethod
def from_flyte_idl(cls, p):
"""
:param flyteidl.admin.workflow_pb2.WorkflowClosure p:
:rtype: WorkflowClosure
"""
return cls(compiled_workflow=_compiler_models.CompiledWorkflowClosure.from_flyte_idl(p.compiled_workflow))
return cls(
compiled_workflow=_compiler_models.CompiledWorkflowClosure.from_flyte_idl(p.compiled_workflow),
created_at=p.created_at.ToDatetime().replace(tzinfo=_timezone.utc) if p.HasField("created_at") else None,
)
For the execute with version part, I simply split the entity name with ":"
, and iteratively loop through possible combinations of entity_name and version with the _looped_fetch_entity
method.
How was this patch tested?
By running pyflyte show-versions remote-[launchplan|workflow|task] {name}
, the versions will be displayed
Code snippet:
from math import sqrt
from flytekit import workflow, task, LaunchPlan
from typing import List, Union
import random
import pdb
@task
def standard_deviation(values: List[float], mu: float) -> float:
variance = sum([(x - mu) ** 2 for x in values])
return sqrt(variance)
@task
def standard_scale(values: List[float], mu: float, sigma: float) -> List[float]:
return [(x - mu) / sigma for x in values]
@task
def mean(values: List[float]) -> float:
return sum(values) / len(values)
@task
def mean2(values: List[float]) -> float:
return sum(values) / len(values)
@task
def generate_data(num_samples: int, seed: int) -> List[float]:
random.seed(seed)
return [random.random() for _ in range(num_samples)]
@workflow
def standard_scale_workflow(values: List[float]) -> List[float]:
mu = mean(values=values)
sigma = standard_deviation(values=values, mu=mu)
return standard_scale(values=values, mu=mu, sigma=sigma)
@workflow
def workflow_with_subworkflow(num_samples: int, seed: int) -> List[float]:
data = generate_data(num_samples=num_samples, seed=seed)
return standard_scale_workflow(values=data)
# pdb.set_trace()
print("Compiled")
# LaunchPlan.CACHE.pop("standard_scale_lp", None)
standard_scale_launch_plan = LaunchPlan.get_or_create(
standard_scale_workflow,
name="standard_scale_lp",
default_inputs={"values": [3.0, 4.0, 5.0]}
)
Example:
pyflyte show-versions remote-task flyteTest.example.mean
One can then run
pyflyte run remote-task flyteTest.example.mean:{version} --inputs ...
Setup process
Screenshots
Check all the applicable boxes
- [ ] I updated the documentation accordingly.
- [v] All new and existing tests passed.
- [v] All commits are signed-off.
Related PRs
Docs link
There is one problem with this
--show-versions
implies you cannot have a task or a workflow with show_versions
as an input
Example
@task
def do(show_versions: bool):
...
which might be ok. But to counter this, I suggested add a subcommand of type
pyflyte run remote-task mytask show-versions
Note the missing --
cc @wild-endeavor what should we do?
If you create a subcommand, like how we do for remote-entities. the table also does not need to be created!
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 83.22%. Comparing base (
66a6018
) to head (5556d00
).
:exclamation: Current head 5556d00 differs from pull request most recent head 68884bd. Consider uploading reports for the commit 68884bd to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## master #2124 +/- ##
==========================================
- Coverage 83.48% 83.22% -0.27%
==========================================
Files 324 300 -24
Lines 24744 23938 -806
Branches 3521 3502 -19
==========================================
- Hits 20658 19922 -736
+ Misses 3453 3388 -65
+ Partials 633 628 -5
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
can we make show-versions
or maybe just versions
a top level command on pyflyte? so just pyflyte versions
instead of pyflyte run
?
can we make
show-versions
or maybe justversions
a top level command on pyflyte? so justpyflyte versions
instead ofpyflyte run
?
Thanks for the advice, this sounds more feasible for me, I've modified my PR to implement this behavior.
I can't get this to run locally actually. running the new command I'm getting
File "/Users/ytong/go/src/github.com/flyteorg/flytekit/flytekit/clis/sdk_in_container/versions.py", line 19, in __init__
super(click.RichGroup, self).__init__(name, h, entity_name, launcher, **kwargs)
related, why is DynamicEntityVersionCommand
inheriting both click.RichGroup
and DynamicEntityLaunchCommand
? why is this related to launch?
@wild-endeavor Hi, sorry for the confusion. DynamicEntityVersionCommand
inherits the DynamicEntityLaunchCommand
because it needs to call the _fetch_entity
method of DynamicEntityLaunchCommand
in list_commands
method, so that it can find the type of the desired entity.
Regarding the error, could you please provide the command you used that triggers the error you mentioned as well as the error log? Thanks