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

refactor: make workflow details generic so it can be used for tasks #96

Merged
merged 2 commits into from
Sep 9, 2020

Conversation

schottra
Copy link
Contributor

@schottra schottra commented Sep 9, 2020

flyteorg/flyte#459

Tasks/Workflows have a lot of similar information. The details view only really differs in whether or not we want to show schedules and what filter we need to use to fetch the executions.
So as a preparatory step for showing task details, this change moves all of the WorkflowDetails logic into a shared set of Entity components. Rendering the details for an entity only requires passing a ResourceIdentifier. The individual view components will be responsible for pulling the correct values from the route and rendering a EntityDetails component using the right resourceType value.

Note While the work has been done to enable showing Task details, it isn't hooked up yet. That will be in a follow-up PR.

  • Added components/Entities folder to hold components which are common across entity types (workflow, launch plan, task, etc..)
  • Moved the components in WorkflowDetails into the new components folder
  • Made EntityDetails components generic based on a passed ResourceIdentifier
    • Sections in the view are conditionally shown based on a set of flag constants which are mapped for each resourceType. The default view is just to show the description of the entity. Workflows will show everything. Tasks will show everything except schedules.
  • Created maps for string constants and executions filters based on resource type.

@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2020

Codecov Report

Merging #96 into single-task-execution will increase coverage by 0.04%.
The diff coverage is 55.55%.

Impacted file tree graph

@@                    Coverage Diff                    @@
##           single-task-execution      #96      +/-   ##
=========================================================
+ Coverage                  67.50%   67.54%   +0.04%     
=========================================================
  Files                        371      372       +1     
  Lines                       5992     6021      +29     
  Branches                     932      945      +13     
=========================================================
+ Hits                        4045     4067      +22     
- Misses                      1947     1954       +7     
Impacted Files Coverage Δ
src/components/Task/TaskDetails.tsx 0.00% <0.00%> (ø)
src/models/Common/types.ts 100.00% <ø> (ø)
src/components/Entities/EntityDetailsHeader.tsx 57.14% <28.57%> (ø)
src/components/Entities/EntityDetails.tsx 48.27% <48.27%> (ø)
src/components/Entities/EntityDescription.tsx 57.14% <50.00%> (ø)
src/components/Entities/EntityExecutions.tsx 63.63% <54.54%> (ø)
...rc/components/Entities/executionFilterGenerator.ts 60.00% <60.00%> (ø)
src/components/Workflow/WorkflowDetails.tsx 60.00% <60.00%> (ø)
src/components/Entities/EntitySchedules.tsx 100.00% <100.00%> (ø)
src/components/Entities/constants.ts 100.00% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1448569...d02feba. Read the comment docs.

@schottra schottra merged commit 9c7ec47 into single-task-execution Sep 9, 2020
@schottra schottra deleted the task-details-view branch September 9, 2020 23:28
Copy link

@podehnal podehnal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

schottra added a commit that referenced this pull request Oct 8, 2020
* refactor: make workflow details generic so it can be used for tasks (#96)

* refactor: make workflow details generic so it can be used for tasks

* chore: cleanup and moving tests over

* feat: adding route and navigation to the task details page (#97)

* Refactor Launch form to use a state machine (#99)

* refactor: filling out details of the state machine for launch

* refactor: checkpoint

* refactor: mostly finished wiring of machine to component state

* refactor: more work to get form component migrated to using machine

* refactor: cleaning up state for selectors

* fix: type error due to patch version upgrade

* refactor: trying a flat state structure

* fix: getting all tests passing again

* chore: cleanup and docs

* chore: pull request feedback

* refactor: Make launch state and components generic (#100)

* refactor: splitting launch machine into two separate types

* refactor: move shared state out to component

* refactor: use composition to create workflow form

* refactor: update usage of LaunchWorkflowForm -> LaunchForm

* chore: cleanup and fix tests

* feat: Add Task support to Launch form (#101)

* feat: add task support in launch components

* test: updating launch form tests to handle task cases

* fix: remaining broken tests

* Cleanup work for launching single task executions. (#102)

* feat: add support for launch tasks in entity details view

* fix: correctly map initial parameters when relaunching

* fix: correct parent name and back link in execution details page

* fix: pass through referenceExecution when relaunching

* test: check rendering of referenceExecution

* test: adding tests for relaunch flow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants