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: render CronSchedule #107

Merged
merged 4 commits into from
Oct 21, 2020
Merged

feat: render CronSchedule #107

merged 4 commits into from
Oct 21, 2020

Conversation

honnix
Copy link
Member

@honnix honnix commented Oct 20, 2020

TL;DR

Render CronSchedule with optional offset value.

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

flyteorg/flyte#529

Follow-up issue

NA

Render CronSchedule
@honnix
Copy link
Member Author

honnix commented Oct 20, 2020

@schottra PTAL.

@@ -54,6 +57,18 @@ export const schedulesTableColumns: KeyedColumnProps[] = [
label: 'frequency',
width: schedulesTableColumnsWidths.frequency
},
{
Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly sure how this is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

This component isn't actually used at the moment. It's left over from the first iteration of the UI which had a very basic view of the schedules (both active and inactive) in a workflow.

@codecov-io
Copy link

codecov-io commented Oct 20, 2020

Codecov Report

Merging #107 into master will increase coverage by 0.94%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   69.37%   70.31%   +0.94%     
==========================================
  Files         383      383              
  Lines        6299     6388      +89     
  Branches      986     1018      +32     
==========================================
+ Hits         4370     4492     +122     
+ Misses       1929     1896      -33     
Impacted Files Coverage Δ
src/components/Launch/constants.ts 0.00% <ø> (ø)
src/common/formatters.ts 99.13% <100.00%> (+3.54%) ⬆️
src/components/Entities/EntitySchedules.tsx 100.00% <100.00%> (ø)
.../components/Executions/TaskExecutionsList/utils.ts 92.85% <0.00%> (-7.15%) ⬇️
src/models/Execution/constants.ts 100.00% <0.00%> (ø)
src/components/Executions/types.ts 100.00% <0.00%> (ø)
src/components/Executions/Tables/constants.ts 100.00% <0.00%> (ø)
src/components/Executions/utils.ts 78.18% <0.00%> (+1.48%) ⬆️
.../components/Executions/Tables/NodeExecutionRow.tsx 97.77% <0.00%> (+6.66%) ⬆️
...components/Executions/useDetailedNodeExecutions.ts 83.33% <0.00%> (+8.33%) ⬆️
... and 4 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 9b54eee...a7150c7. Read the comment docs.

@@ -54,6 +57,18 @@ export const schedulesTableColumns: KeyedColumnProps[] = [
label: 'frequency',
width: schedulesTableColumnsWidths.frequency
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

This component isn't actually used at the moment. It's left over from the first iteration of the UI which had a very basic view of the schedules (both active and inactive) in a workflow.

src/common/formatters.ts Outdated Show resolved Hide resolved
src/common/test/formatters.spec.ts Show resolved Hide resolved
src/components/Entities/EntitySchedules.tsx Outdated Show resolved Hide resolved
@honnix
Copy link
Member Author

honnix commented Oct 21, 2020

@schottra I fixed the offset rendering according to what we agreed. PTAL again, thanks.

@schottra
Copy link
Contributor

@honnix Looks good. Thanks for implementing this!

@service-github-lyft-semantic-release
Copy link
Contributor

🎉 This PR is included in version 0.15.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@@ -7,5 +7,6 @@ export const launchPlansTableColumnWidths = {
export const schedulesTableColumnsWidths = {
active: 80,
frequency: 300,
offset: 300,
Copy link
Member Author

Choose a reason for hiding this comment

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

I made a mistake here. :( This is not needed.

honnix added a commit to honnix/flyteconsole that referenced this pull request Oct 22, 2020
This was added mistakenly when working on flyteorg#107 .
@honnix honnix mentioned this pull request Oct 22, 2020
8 tasks
schottra pushed a commit that referenced this pull request Oct 22, 2020
This was added mistakenly when working on #107 .
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