feat: show launchplan in execution table #738
Conversation
|
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
Codecov Report
@@ Coverage Diff @@
## master #738 +/- ##
==========================================
+ Coverage 66.73% 66.79% +0.06%
==========================================
Files 464 465 +1
Lines 11390 11411 +21
Branches 2104 2108 +4
==========================================
+ Hits 7601 7622 +21
Misses 3789 3789
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Hey thanks for this PR; looking through the code this looks largely okay my only concern is around how we want to handle archived launch plans (in general) so I've forwarded this PR to our design/UX team and we'll try to get some feedback in the next day or two 👍 |
|
@jsonporter I actually think it makes sense to show launchplan, as you trigger launchplans and they become executions. For a single workflow, there could be multiple launch plans. from utlity pov, i like this PR +1 |
| @@ -10,6 +10,10 @@ export const useStyles = makeStyles((theme: Theme) => ({ | |||
| flexBasis: workflowExecutionsTableColumnWidths.name, | |||
| whiteSpace: 'normal', | |||
| }, | |||
| columnLaunchPlan: { | |||
There was a problem hiding this comment.
Wondering how this would handle a really large string; could we add something like overflow:hidden here?
There was a problem hiding this comment.
You are right, it's ugly without overflow:hidden
Fixed it.
Screen.Recording.2023-04-06.at.9.38.47.PM.mov
|
In the previous Figma design proposal.(shown down below) , the access to the launchplan was placed at the same level as "archive" and "view I/O" , as a "hovering over icon". However, the question remains: how prominent should the launchplan's access be? If our goal is to display the launchplan's relevant information (such as name and version) upfront, then the proposal in this PR seems reasonable.
|
The pain point that I am addressing is the need for additional clicks to figure out which launch plan is used when executing a workflow. Currently, it's possible to figure that out by opening each execution page (with a few extra clicks). But it's becoming painful when there are multiple launch plans executing the same workflow. Having this as a column greatly helps to minimize friction. |
Signed-off-by: Pradithya Aria Pura <[email protected]>
Signed-off-by: Pradithya Aria Pura <[email protected]>
Signed-off-by: Pradithya Aria Pura <[email protected]>
Signed-off-by: Pradithya Aria Pura <[email protected]>
Signed-off-by: Pradithya Aria Pura <[email protected]>
Signed-off-by: Pradithya Aria Pura <[email protected]>
|
This PR seems like an improvement to me. In general, bringing information up, not hiding it behind clicks, is a good thing. It becomes a negative only when the additional info is distracting, not related to primary task of the view, or it makes the view too complex and hard to read. Neither of those concerns appear to apply in this case. @jsonporter can you say more about this: "my only concern is around how we want to handle archived launch plans (in general)..." |
|
Yeah just that "archiving" a launch plan is a little odd in that it doesn't actually prevent executions (ie, you can launch a WF from an archived launch plan). That said, we discussed the edge cases and they seem fine; i'm good with this PR if everyone else is 👍 |
|
Congrats on merging your first pull request! 🎉 |
|
🎉 This PR is included in version 1.6.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
TL;DR
Display Launch Plan name in the workflow execution table and link it to the launch plan details page.
Type
Are all requirements met?
Complete description
Display Launch Plan name in the workflow execution table and link it to the launch plan details page.
Screen.Recording.2023-04-01.at.9.57.06.AM.mov
Tracking Issue
flyteorg/flyte#3555
Follow-up issue
NA