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

Add condition to generation of dynamic job spec #109

Merged
merged 3 commits into from
May 8, 2020

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented May 7, 2020

TL;DR

Please see the issue for a proper description.

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

If a launch plan/subworkflow node does not have outputs, currently there is no dynamic job spec created. This was the main issue this PR is meant to address.

Also discovered that if the workflow/launchplan yielded in a dynamic node does not have outputs, but does have inputs, those inputs would not be generated. This was from improper indentation from the first time this code was created.

This code needs to be cleaned up... a lot. See the follow-up issue.

Tracking Issue

flyteorg/flyte#282

Follow up issue

flyteorg/flyte#226

…launch plan node for a workflow with no outputs and no inputs
@feiyu-meng
Copy link

Thanks!

@codecov-io
Copy link

Codecov Report

Merging #109 into release-0.7 will increase coverage by 0.03%.
The diff coverage is 95.23%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release-0.7     #109      +/-   ##
===============================================
+ Coverage        82.24%   82.28%   +0.03%     
===============================================
  Files              208      208              
  Lines            13266    13307      +41     
  Branches          1106     1109       +3     
===============================================
+ Hits             10911    10950      +39     
- Misses            2092     2094       +2     
  Partials           263      263              
Impacted Files Coverage Δ
tests/flytekit/unit/models/test_dynamic_wfs.py 94.59% <95.00%> (+0.47%) ⬆️
flytekit/common/tasks/sdk_dynamic.py 95.00% <100.00%> (+0.05%) ⬆️

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 07f8e80...6d42e4a. Read the comment docs.

@wild-endeavor wild-endeavor merged commit ec167bd into release-0.7 May 8, 2020
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