flytekit icon indicating copy to clipboard operation
flytekit copied to clipboard

Make additional execution info configurable via plugin

Open ddl-rliu opened this issue 1 year ago • 3 comments

Tracking issue

https://github.com/flyteorg/flyte/issues/5641

Why are the changes needed?

In certain deployments, it is useful to augment the console output of pyflyte run to report some additional info. In our case, this additional info includes a second url with agent-specific info about an execution (agent here refers to a custom agent we are using to run our flyte executions).

So, this PR enhances the console output of pyflyte run to be "pluggable" – this way, additional info here can be configured via plugin. By default, no additional info is supplied, so the behavior is unchanged for most cases.

What changes were proposed in this pull request?

Makes the additional execution info pluggable. This can be considered as an extension to https://github.com/flyteorg/flytekit/pull/2039 which enabled flytekit to be pluggable.

How was this patch tested?

Setup process

pip install <library implementing custom flytekit plugin> git+ssh://[email protected]/ddl-rliu/flytekit@rliu-configurable-execution-console-url

Screenshots

$ pyflyte run --remote wf.py wf --a 1 --b 1
...
[✔] Go to https://.../domains/development/executions/amazingly-many-cockatoo-u99k to see the
execution in the console.
    Go to https://<url-with-additional-info>/amazingly-many-cockatoo-u99k to see the execution in Domino.

Check all the applicable boxes

  • [ ] I updated the documentation accordingly.
  • [ ] All new and existing tests passed.
  • [x] All commits are signed-off.

Related PRs

Docs link

ddl-rliu avatar Aug 07 '24 22:08 ddl-rliu

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.12%. Comparing base (d802c7e) to head (5ccd864). Report is 29 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2661       +/-   ##
===========================================
- Coverage   92.53%   80.12%   -12.42%     
===========================================
  Files          27      273      +246     
  Lines        1206    23283    +22077     
  Branches        0     4014     +4014     
===========================================
+ Hits         1116    18655    +17539     
- Misses         90     3964     +3874     
- Partials        0      664      +664     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 07 '24 22:08 codecov[bot]

I’m concerned with the precedence this PR sets for extending console printing. There are many places in flytekit that prints to the console. Where do we draw the line for console printing extensions?

Overall, I prefer not to have the configuration plugin be used to customize printing.

thomasjpfan avatar Aug 20 '24 23:08 thomasjpfan

@thomasjpfan Thanks for the feedback, it's a valid concern. As mentioned in the issue, an alternative we considered is to maintain a custom CLI "custom-pyflyte-wrapper-cli" which wraps pyflyte, additionally including the various execution info/console output that we desire.

We have not started this effort, but it could be the next alternative to making the execution info console output pluggable.

ddl-rliu avatar Aug 22 '24 17:08 ddl-rliu