-
Notifications
You must be signed in to change notification settings - Fork 304
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.
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
Introduce flyte Decks into flytekit #859
Conversation
Signed-off-by: Kevin Su <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #859 +/- ##
==========================================
- Coverage 86.47% 86.47% -0.01%
==========================================
Files 234 239 +5
Lines 22875 23100 +225
Branches 2577 2604 +27
==========================================
+ Hits 19782 19975 +193
- Misses 2655 2680 +25
- Partials 438 445 +7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's still wip but just at first glance...
- If we're going to have templating in the html, should we consider something like jinja2?
- Could you summarize the user-facing API somewhere.
- How will this work for dynamic tasks? Dynamic tasks are compiled at run-time - will these Deck objects interfere with compilation?
- How does this work with cached stuff?
@jsonporter could you take a look at the html page in the PR? just in case you have any thoughts.
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Few updates,
|
Signed-off-by: Kevin Su <[email protected]>
This is looking much better now |
Signed-off-by: Kevin Su <[email protected]>
Overall I love the PR, this is simple. But, I think we should support a simple |
cc @pradithya / @eapolinario |
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
tests/flytekit/unit/types/structured_dataset/test_structured_dataset_workflow.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
cc @eapolinario we discussed 2 things right?
|
Signed-off-by: Kevin Su <[email protected]>
Few updates,
|
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
flytekit/core/context_manager.py
Outdated
@@ -79,6 +81,7 @@ class Builder(object): | |||
attrs: typing.Dict[str, typing.Any] | |||
working_dir: typing.Union[os.PathLike, utils.AutoDeletingTempDir] | |||
checkpoint: typing.Optional[Checkpoint] | |||
decks: List |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
list of?
Signed-off-by: Kevin Su <[email protected]>
* Introduce flyte Decks into flytekit Signed-off-by: Kevin Su <[email protected]> * Updated deck interface Signed-off-by: Kevin Su <[email protected]> * Updated deck interface Signed-off-by: Kevin Su <[email protected]> * Updated dependency Signed-off-by: Kevin Su <[email protected]> * Updated comment Signed-off-by: Kevin Su <[email protected]> * Fixed tests Signed-off-by: Kevin Su <[email protected]> * Fixed tests Signed-off-by: Kevin Su <[email protected]> * Deck plugin Signed-off-by: Kevin Su <[email protected]> * Refactor deck interface Signed-off-by: Kevin Su <[email protected]> * Updated tests Signed-off-by: Kevin Su <[email protected]> * Updated tests Signed-off-by: Kevin Su <[email protected]> * Updated tests Signed-off-by: Kevin Su <[email protected]> * Updated tests Signed-off-by: Kevin Su <[email protected]> * Updated tests Signed-off-by: Kevin Su <[email protected]> * Updated tests Signed-off-by: Kevin Su <[email protected]> * Fixed tests Signed-off-by: Kevin Su <[email protected]> * Lint fixed Signed-off-by: Kevin Su <[email protected]> * Addressed comment Signed-off-by: Kevin Su <[email protected]> * Fixed tests Signed-off-by: Kevin Su <[email protected]> * Fixed plugin tests Signed-off-by: Kevin Su <[email protected]> * Fixed spark plugin tests Signed-off-by: Kevin Su <[email protected]> * Tests fixed Signed-off-by: Kevin Su <[email protected]> * Add deck plugin to ci Signed-off-by: Kevin Su <[email protected]> * Fixed tests Signed-off-by: Kevin Su <[email protected]> * More tests Signed-off-by: Kevin Su <[email protected]> * Fix spark tests Signed-off-by: Kevin Su <[email protected]> * Fix lint Signed-off-by: Kevin Su <[email protected]> * address comments Signed-off-by: Kevin Su <[email protected]> * address comments Signed-off-by: Kevin Su <[email protected]> * Fixed tests error Signed-off-by: Kevin Su <[email protected]> * Fixed tests error Signed-off-by: Kevin Su <[email protected]> * add example and update type hint Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Mike Zhong <[email protected]>
Signed-off-by: Kevin Su [email protected]
TL;DR
Allow user to visualize dataframe, preview markdown file on flyteconsole
Note: now It only works for local execution
To run an example:
pip install git+https://github.com/flyteorg/flytekit@deck && python example_deck.py
output will be like
We should add some metadata in flyteidl to make flyteconsole be able to find those HTML files
Type
Are all requirements met?
Complete description
renderer
interfaceFrameRenderer
,MarkdownRenderer
,FrameProfilingRenderer
renderer
Tracking Issue
flyteorg/flyte#2175
Follow-up issue
NA