FlyteCoPilot: Raw container support [Alpha] #107
Conversation
| :param metadata_format: Format in which the metadata will be available for the script | ||
| """ | ||
|
|
||
| # Set as class fields which are used down below to configure implicit |
There was a problem hiding this comment.
these aren't class fields though.
There was a problem hiding this comment.
self.input_data_dir = input_data_dir self.output_data_dir = output_data_dir self.metadata_format = metadata_format
| "This is deprecated!" | ||
| ) | ||
|
|
||
| # Here we set the routing_group, catalog, and schema as implicit |
There was a problem hiding this comment.
leftover comment from presto?
| # parameters for caching purposes | ||
| i = _interface.TypedInterface(inputs=types_to_variable(inputs), outputs=types_to_variable(outputs)) | ||
|
|
||
| # TODO create custom proto to store data dir and other things |
There was a problem hiding this comment.
no not remove, add the custom proto I guess?
| from flytekit.sdk.types import Types | ||
|
|
||
|
|
||
| def test_raw_container_task_definition(): |
There was a problem hiding this comment.
can you do a test in your test harness, and maybe add one here, where you have a workflow with two of these raw container tasks and the second one consumes the output of the first one? Or is that not possible to do?
There was a problem hiding this comment.
it is possible, good point. I actually have a real workflow, just scroll up. But i will write an test workflow too
There was a problem hiding this comment.
actually i cannot write a test today, as it will execute it locally, which we do not want
|
|
||
| class SdkRawContainerTask(_base_task.SdkTask): | ||
| """ | ||
| This class includes the logic for building a task that executes as a Presto task. |
There was a problem hiding this comment.
Can we add a few paragraphs of documentation here or somewhere else on 1) how to use this and 2) how it works?
For 1) I've left you a page to fill in here: https://github.com/lyft/flytesnacks/blob/cookbook/cookbook/workflows/recipe_5/index.rst
Feel free to check out that branch and commit... or make your own PR if you want the street cred.
| ) | ||
|
|
||
| echo = SdkRawContainerTask( | ||
| input_data_dir="/var/flyte/inputs", | ||
| inputs={"x": Types.Integer}, | ||
| image="alpine", | ||
| command=["cat", "/var/flyte/inputs.json"], | ||
| command=["sh" "-c", "ls /var/flyte/inputs; cat /var/flyte/inputs/inputs.json"], |
There was a problem hiding this comment.
why not make this a bit more complicated? echo '3 + 5' | bc > output_file. where 3 and or 5 comes from an input?
There was a problem hiding this comment.
the previous step is complicated
There was a problem hiding this comment.
if you want i can modify this to produce an output?
|
Also, bump version. |
|
Also, it looks like python 2.7 tests are failing. let's just stop running them. it's in travis.yml. I thought we had done that already but i guess not. |
flytekit/__init__.py
Outdated
| @@ -2,4 +2,4 @@ | |||
|
|
|||
| import flytekit.plugins | |||
|
|
|||
| __version__ = '0.9.1' | |||
| __version__ = '0.9.3' | |||
There was a problem hiding this comment.
i made this 0.9.3 cuz I thought katrina's change for single-task would go in first but I think this will go in first actually.
Codecov Report
@@ Coverage Diff @@
## master #107 +/- ##
==========================================
+ Coverage 81.37% 81.39% +0.02%
==========================================
Files 215 217 +2
Lines 14027 14145 +118
Branches 1146 1159 +13
==========================================
+ Hits 11415 11514 +99
- Misses 2341 2349 +8
- Partials 271 282 +11
Continue to review full report at Codecov.
|
TL;DR
This change adds a new type of task called SDKRawContainer task. Currently this is in Alpha, but eventually will become the base task for all container executions on K8s.
Type
Are all requirements met?
Complete description
Refer to the Design doc at https://docs.google.com/document/d/1ZsCDOZ5ZJBPWzCNc45FhNtYQOxYHz0PAu9lrtDVnUpw/edit?ts=5ea7f579#
Tracking Issue
flyteorg/flyte#297
Follow-up issue
NA