Skip to content
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

feat: show io urls & flyteremote usage #824

Merged
merged 1 commit into from
Oct 19, 2023
Merged

feat: show io urls & flyteremote usage #824

merged 1 commit into from
Oct 19, 2023

Conversation

ursucarina
Copy link
Contributor

@ursucarina ursucarina commented Oct 19, 2023

TL;DR

  • Shows Context Panel IO URLs
  • Shows Context Panel IO FlyteRemote usage
  • Shows Context Panel node execution logs
image image
image image

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

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

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

@ursucarina ursucarina requested review from a team, FrankFlitton and jsonporter and removed request for a team October 19, 2023 02:05
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Attention: 27 lines in your changes are missing coverage. Please review.

Comparison is base (ce46159) 63.34% compared to head (916e26c) 63.25%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #824      +/-   ##
==========================================
- Coverage   63.34%   63.25%   -0.09%     
==========================================
  Files         527      528       +1     
  Lines       13400    13432      +32     
  Branches     2554     2559       +5     
==========================================
+ Hits         8488     8497       +9     
- Misses       4912     4935      +23     
Files Coverage Δ
...cutionDetails/NodeExecutionDetailsPanelContent.tsx 78.18% <0.00%> (+0.72%) ⬆️
...nDetails/NodeExecutionTabs/NodeExecutionInputs.tsx 66.66% <33.33%> (-11.12%) ⬇️
...Details/NodeExecutionTabs/NodeExecutionOutputs.tsx 66.66% <33.33%> (-11.12%) ⬇️
...rc/components/Executions/ExecutionDetails/utils.ts 54.28% <0.00%> (-3.30%) ⬇️
...s/Executions/ExecutionDetails/ExecutionNodeURL.tsx 43.75% <43.75%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jpvotta
Copy link

jpvotta commented Oct 19, 2023

@pingsutw how does this work with maptask (subtasks)? I believe we are doing some parsing in the FE in order to allocate the input/output list elements into inputs/outputs of each subtask respectively. I am guessing that we would want to include these new visual elements in the maptask parent (since the literalmap will include the full list of inputs/outputs including all subtasks), but not for each subtask (since this would be identical to the info in the parent).

@pingsutw
Copy link
Member

I believe we are doing some parsing in the FE in order to allocate the input/output list elements into inputs/outputs of each subtask respectively.

Right, so there is no flyte url for sub task. we can just show it only in the parent for now.
btw, UI looks pretty good now, thank you so much.

one nit: we should not show the copy URL and code snippet if the input or output is empty.

@pingsutw
Copy link
Member

cc @wild-endeavor

@ursucarina ursucarina merged commit 4adc6e3 into master Oct 19, 2023
9 of 11 checks passed
@ursucarina ursucarina deleted the carina/iouri branch October 19, 2023 17:19
@flyte-bot
Copy link
Collaborator

🎉 This PR is included in version 1.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants