I'm debugging an issue related to our adoption of ...
# announcements
d
I'm debugging an issue related to our adoption of the recent change in 0.8.7 to skipped fan in dependencies. I see the tests cover the case where a direct dependency that doesn't fire an output works fine: https://github.com/dagster-io/dagster/blob/master/python_modules/dagster/dagster_tests/core_tests/test_pipeline_execution.py#L968-L996 However in my case, if the fan in solid takes an
is_required=False
input from a solid that is itself skipped (due to it not getting an upstream solid's optional output), then dagster still skips the fan in solid. It seems this is what's happening, still debugging tho. Can anyone confirm? (Note: Other inputs to the fan in solid were supplied so it's not skipping because all inputs were not provided.)
I can't get the tests to pass locally but here's how I would rewrite the existing tests I linked to expose this issue:
Copy code
def test_multi_dep_optional():
    @lambda_solid
    def ret_one():
        return 1

    @solid(output_defs=[OutputDefinition(name='skip', is_required=False)])
    def skip(_):
        return
        yield  # pylint: disable=unreachable

    @solid(output_defs=[OutputDefinition(name='skip', is_required=False)])
    def skip_downstream(_, skip_upstream):
        return
        yield  # pylint: disable=unreachable

    @solid
    def collect(_, items):
        return items

    @pipeline
    def test_remaining():
        collect([ret_one(), skip()])

    result = execute_pipeline(test_remaining)
    assert result.success
    assert result.result_for_solid('collect').output_value() == [1]

    @pipeline
    def test_all_skip():
        collect([skip(), skip(), skip()])

    result = execute_pipeline(test_all_skip)
    assert result.success
    assert result.result_for_solid('collect').skipped

    @pipeline
    def test_skipped_upstream():
        collect([ret_one(), skip_downstream(skip())])

    result = execute_pipeline(test_skipped_upstream)
    assert result.success
    assert result.result_for_solid('collect').output_value() == [1]
If someone with a working test env can please try running the above and let me know if it passes, that'll be great
a
ya you are correct this test does not pass
thanks for the report and the test case, should be able to get in a fix for the release next week (missed todays)
apologies for the fairly negligent oversight
d
Thanks @alex! 👍
Hey @alex just wanted to ping about this. Didn't notice an issue for it on github, would it be helpful for me to make one to ensure you guys can track this?
a
just got the diff out for review so should be in for the next release
d
Woot!!!
You are awesome
🙏