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

Allow empty outputs #292

Merged
merged 3 commits into from
Aug 2, 2016
Merged

Allow empty outputs #292

merged 3 commits into from
Aug 2, 2016

Conversation

macheins
Copy link
Contributor

Flux expects at least one output if fails on not defined or empty spec.outputs. Since empty outputs were supported in all 3.0.0-dev releases this breaks a lot of recent code.

@macheins
Copy link
Contributor Author

FYI the problem was introduced here.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 45.43% when pulling 7f8c1e0 on macheins:allow-empty-outputs into 302aa05 on Parsely:master.

@@ -194,6 +194,8 @@ def _spec_to_flux_dict(spec):
shell_object = spec.component_object.shell
flux_dict['constructorArgs'].append([shell_object.execution_command,
shell_object.script])
if not spec.outputs or len(spec.outputs) == 0:
Copy link
Member

Choose a reason for hiding this comment

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

Super minor nitpick, but could you please get rid of the or len(spec.outputs) == 0 from this line? It's unnecessary, because if spec.outputs is empty, not spec.outputs will be true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your are right. Done.

@dan-blanchard
Copy link
Member

Thanks for the pull request!

@dan-blanchard dan-blanchard added bug and removed backlog labels Aug 1, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 45.399% when pulling 1daabea on macheins:allow-empty-outputs into 6192b66 on Parsely:master.

Removed unreachable check
@coveralls
Copy link

coveralls commented Aug 2, 2016

Coverage Status

Coverage decreased (-0.06%) to 45.399% when pulling 4edb0ae on macheins:allow-empty-outputs into 6192b66 on Parsely:master.

@dan-blanchard dan-blanchard merged commit 00bafed into pystorm:master Aug 2, 2016
@macheins macheins deleted the allow-empty-outputs branch August 2, 2016 15:17
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.

4 participants