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

Mapped Tasks not showing cache status correctly. #712

Merged
merged 3 commits into from
Mar 7, 2023

Conversation

james-union
Copy link
Contributor

https://github.com/flyteorg/flyteconsole/issues/644

Two issues
(1) It renders cache status incorrectly because it depended on the index of the logs instead of the id. While running, the logs are not in the original order (they are in order of queued/success instead of original order). So need to find correct cache status based on id
(2) the useStyles/hooks should not be used after component is rendered. It broke the app when the execution status is updated. I managed to place it before any render

To reproduce

For this workflow,
https://development.uniondemo.run/console/projects/flytesnacks/domains/development/workflows/map-task.my_cached_map_workflow_1?duration=all

  • Click Launch Workflow button
  • For the parameter, we need to mix some cached numbers and non-cached numbers to reproduce this issue. Right now, 0 ~ 8 are cached. (This will be changed if someone run this more). So for example, if 0 ~ 8 are cached, we can set the param as [9,1,0,12,11,3]
  • Then while running the mapped task, if you go to the Map Execution panel on the right side (by navigating to the execution details and and click the first Map task), you will see some queued & succeeded tasks. Actually the problem is the icons next to the task names. If those are already queued, then it should be the refresh icon which identifies already cached.
  • To reproduce the second issue, you should wait until the Map task is completed while open this right panel. Once it is updated, the right panel will be broken.

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 flyteorg/flyte#644

@james-union james-union self-assigned this Mar 7, 2023
@james-union james-union requested a review from ursucarina March 7, 2023 12:16
@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #712 (15854b1) into master (c5cc902) will increase coverage by 0.09%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master     #712      +/-   ##
==========================================
+ Coverage   66.92%   67.01%   +0.09%     
==========================================
  Files         455      455              
  Lines       11271    11269       -2     
  Branches     2077     2076       -1     
==========================================
+ Hits         7543     7552       +9     
+ Misses       3728     3717      -11     
Impacted Files Coverage Δ
...ents/common/MapTaskExecutionsList/TaskNameList.tsx 88.88% <60.00%> (-4.22%) ⬇️
...s/console/src/components/hooks/useDataRefresher.ts 100.00% <0.00%> (+41.37%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ursucarina ursucarina merged commit 2a40b09 into master Mar 7, 2023
@ursucarina ursucarina deleted the james/mapped-cache-status branch March 7, 2023 16:52
@flyte-bot
Copy link
Collaborator

🎉 This PR is included in version 1.4.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

4nalog pushed a commit that referenced this pull request Mar 30, 2023
* fix: cache status logs indexing mismatch and react rendering cycle issue

Signed-off-by: James <[email protected]>

* fix: mapped task cache status

Signed-off-by: James <[email protected]>

---------

Signed-off-by: James <[email protected]>
Co-authored-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>
jsonporter added a commit that referenced this pull request Mar 30, 2023
* fix: type error on invalid nested json input

Signed-off-by: 4nalog <[email protected]>

* LaunchForm RJSF Form issue (#692)

* fix: launchform

Signed-off-by: James <[email protected]>

* fix: stuck issue

Signed-off-by: James <[email protected]>

* fix: on form change

Signed-off-by: James <[email protected]>

* fix: merge master into branch

Signed-off-by: James <[email protected]>

---------

Signed-off-by: James <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* chore: bump minor version

Signed-off-by: 4nalog <[email protected]>

* fix: package version to 1.4.2 (#700)

Signed-off-by: Jason Porter <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: project settings dashbboard tests (#701)

* fix: project settings dashbboard tests

Signed-off-by: 4nalog <[email protected]>

* chore: remove .only

Signed-off-by: 4nalog <[email protected]>

* refactor: remove empty waitFor

Signed-off-by: 4nalog <[email protected]>

---------

Signed-off-by: 4nalog <[email protected]>

* fix: upgrading node version to 18 (#703)

Upgrading node version to 18

Signed-off-by: Jason Porter <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: revert node version (#704)

* fix: revert node version

Signed-off-by: Carina Ursu <[email protected]>

* Revert "fix: upgrading node version to 18 (#703)"

This reverts commit e5f9a8b.

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: deployment optimization (#706)

* fix: deployment optimization

Signed-off-by: Carina Ursu <[email protected]>

* clean up node modules and one file that we think is not needed

Signed-off-by: Yee Hing Tong <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Co-authored-by: Yee Hing Tong <[email protected]>

* fix: upgrade release node version (#707)

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: update chalk, add semantic-release test cmd (#708)

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* chore: implement logic to handle multiple keys for nested data classes

Signed-off-by: 4nalog <[email protected]>

* chore: fix update_npmversion (#709)

* chore: fix update_npmversion

Signed-off-by: Carina Ursu <[email protected]>

* chore: edits

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* Mapped Tasks not showing cache status correctly. (#712)

* fix: cache status logs indexing mismatch and react rendering cycle issue

Signed-off-by: James <[email protected]>

* fix: mapped task cache status

Signed-off-by: James <[email protected]>

---------

Signed-off-by: James <[email protected]>
Co-authored-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: cleanup, passthrough runtime variables (#710)

fix: passthrough runtime variables

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: update_npmversion (#713)

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: fix sed makefile error (#714)

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* FE: Update flyteconsole to Node 18 (#717)

fix: nodejs 18 upgrade

Signed-off-by: James <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* chore: allow complex workflow names (#715)

* chore: allow complex workflow names

Signed-off-by: Carina Ursu <[email protected]>

* chore: fix buold:watch

Signed-off-by: Carina Ursu <[email protected]>

* chore: yarn.lock

Signed-off-by: Carina Ursu <[email protected]>

* chore: upgrade package

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>

* chore: show correct app version in info  (#716)

* chore: show correct app version in info

Signed-off-by: Carina Ursu <[email protected]>

* chore: update ver

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: left nav doesn't accurately update on  workflow version page (#718)

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* feat: differentiate between cache disabled and cache put failure (#719)

fix: add icon for put failure

Signed-off-by: James <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* chore: fix formatting

Signed-off-by: 4nalog <[email protected]>

* fix: add material-ui class name seed (#721)

* chore: add material-ui class name seed

Signed-off-by: Carina Ursu <[email protected]>

* fix: overflow of modal

Signed-off-by: James <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: James <[email protected]>
Co-authored-by: James <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* The rendering of node status in a dynamic workflow is not functioning correctly (#723)

fix: dynamic nodes status

Signed-off-by: James <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: backfill index on execution task logs (#725)

* chore: backfill index on execution task logs

Signed-off-by: Carina Ursu <[email protected]>

* chore: add comments

Signed-off-by: Carina Ursu <[email protected]>

* chore: bump version

Signed-off-by: Carina Ursu <[email protected]>

* chore: remove backfill

Signed-off-by: Carina Ursu <[email protected]>

* chore: lint

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: release fail (#726)

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* Install deps directly in checks.yml (#728)

Signed-off-by: eduardo apolinario <[email protected]>
Co-authored-by: eduardo apolinario <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: show correct i/o in details panel  (#727)

* chore: show correct i/o in details panel

Signed-off-by: Carina Ursu <[email protected]>

* chore: bump version

Signed-off-by: Carina Ursu <[email protected]>

* chore: feedback

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* chore: task observability (#720)

* chore: tlro basic setup

Signed-off-by: Carina Ursu <[email protected]>

* progress checkin

* Stable state refactored to use generic naming

* updated types

* working before updating routes

* chore: progress

Signed-off-by: Carina Ursu <[email protected]>

* chore: remove debugger

Signed-off-by: Carina Ursu <[email protected]>

* chore: allow custom class in execution actions

Signed-off-by: Carina Ursu <[email protected]>

* chore: lint fix

Signed-off-by: Carina Ursu <[email protected]>

* chore: bump console

Signed-off-by: Carina Ursu <[email protected]>

* chore: up package version

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Co-authored-by: Jason Porter <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* chore: fix contextual menu action buttons color (#730)

* chore: fix contextual menu action buttons color

Signed-off-by: Carina Ursu <[email protected]>

* chore: up console version

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* chore: add build:watch to all packages (#731)

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* Add REJECT support for ApprovedCondition for GateNodes (#733)

* fix: approveCondition reject

Signed-off-by: James <[email protected]>

* fix: approve button

Signed-off-by: James <[email protected]>

---------

Signed-off-by: James <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* gate node in dynamic task (#729)

* fix: gate node in dynamic

Signed-off-by: James <[email protected]>

* fix: upgraded version

Signed-off-by: James <[email protected]>

* fix: compiledNode in PausedTasksComponent and ExecutionDetailsActions

Signed-off-by: James <[email protected]>

* fix: upgrade version

Signed-off-by: James <[email protected]>

* chore: lockfile

Signed-off-by: Carina Ursu <[email protected]>

* fix: remove caching for workflow closure

Signed-off-by: James <[email protected]>

* fix: break link between cache and context state

Signed-off-by: James <[email protected]>

---------

Signed-off-by: James <[email protected]>
Signed-off-by: Carina Ursu <[email protected]>
Co-authored-by: Carina Ursu <[email protected]>
Signed-off-by: 4nalog <[email protected]>

* fix: comments

Signed-off-by: 4nalog <[email protected]>

---------

Signed-off-by: 4nalog <[email protected]>
Signed-off-by: James <[email protected]>
Signed-off-by: Jason Porter <[email protected]>
Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: eduardo apolinario <[email protected]>
Co-authored-by: james-union <[email protected]>
Co-authored-by: Jason Porter <[email protected]>
Co-authored-by: Carina Ursu <[email protected]>
Co-authored-by: Yee Hing Tong <[email protected]>
Co-authored-by: James <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Co-authored-by: eduardo apolinario <[email protected]>
Co-authored-by: Jason Porter <[email protected]>
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.

3 participants