dagster-dbt: Surface dbt CLI failures during selection (prevents StopIteration in multiprocess) Fixes #33380

[vidiyala99]

Fixes #33380

Summary

Fixes an issue where dbt CLI failures during _select_unique_ids_from_cli were silently swallowed, leading to empty metadata and downstream StopIteration / RuntimeError during multiprocess execution.

Root cause

When DbtCliInvocation failed (non-zero exit), the code continued processing _stream_stdout() without checking is_successful(). This produced incomplete/empty unique_id sets, causing failures later in repository reconstruction under multiprocess_executor.

Fix

• After consuming _stream_stdout(), explicitly check invocation.is_successful()
• If the invocation failed:
  • raise invocation.get_error() when available
  • otherwise raise a fallback RuntimeError("dbt invocation failed but no error was captured")

Tests

Added comprehensive unit tests covering:

• successful parsing with mixed stdout events
• empty / malformed stdout events
• failure with explicit error from dbt CLI
• failure without error object

Impact

• Prevents misleading StopIteration / generator errors
• Surfaces true dbt CLI failures during concurrent execution
• Improves debuggability in dagster dev with multiprocess_executor

[greptile-apps]

Greptile Summary

This PR fixes a bug in _select_unique_ids_from_cli where a failed dbt CLI invocation silently returned an empty/incomplete set of unique IDs, causing downstream StopIteration or RuntimeError during multiprocess execution. The fix adds an is_successful() check after consuming _stream_stdout() and raises the captured error (or a fallback RuntimeError) when the invocation fails.

Confidence Score: 5/5

Safe to merge — the fix correctly surfaces previously-swallowed dbt CLI errors with no behavioral regressions on the success path.

The change is minimal and targeted. The _error_messages instance field is populated during the first _stream_stdout() call and persists when is_successful() calls _stream_stdout() again on a closed pipe, so failure detection remains correct. The raise err or RuntimeError(...) idiom is valid Python. Tests cover all four relevant cases: success, empty/malformed events, failure with explicit error, and failure without error. No P0/P1 findings.

No files require special attention.

Important Files Changed

Filename	Overview
python_modules/libraries/dagster-dbt/dagster_dbt/utils.py	Adds is_successful() check after consuming _stream_stdout() to surface dbt CLI failures instead of silently returning an incomplete unique_id set
python_modules/libraries/dagster-dbt/dagster_dbt_tests/core/test_utils.py	New test file with 5 unit tests covering success, empty/malformed events, failure with error, failure without error, and partial stdout failure scenarios

Sequence Diagram

sequenceDiagram
    participant S as select_unique_ids
    participant CLI as _select_unique_ids_from_cli
    participant INV as DbtCliInvocation
    participant PROC as dbt Process

    S->>CLI: call (dbt Fusion path)
    CLI->>INV: DbtCliResource(...).cli(cmd)
    INV->>PROC: spawn process
    CLI->>INV: _stream_stdout()
    INV->>PROC: read stdout (generator)
    PROC-->>INV: raw events; _error_messages populated
    Note over INV: stdout pipe closed after generator exhausted
    CLI->>INV: is_successful()
    INV->>INV: _stream_stdout() → empty (stdout closed)
    INV->>PROC: process.wait() → exit code
    INV-->>CLI: True / False
    alt is_successful() == False
        CLI->>INV: get_error()
        INV-->>CLI: Exception or None
        CLI->>S: raise err or fallback RuntimeError
    else is_successful() == True
        CLI->>S: return unique_ids - {None}
    end

Loading

_{Reviews (2): Last reviewed commit: "test(dagster-dbt): cover partial stdout ..." | Re-trigger Greptile}

[greptile-apps]

_{3 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[vidiyala99]

Hi @OwenKephart — this PR has been open since Feb 22 and the linked bug (#33380) is still open. It's a small fix to dagster-dbt that surfaces dbt CLI failures during _select_unique_ids_from_cli instead of silently swallowing them, preventing misleading StopIteration errors during multiprocess execution. Automated review gave it a 5/5. Would appreciate a look when you get a chance!

[cmpadden]

The main change in python_modules/libraries/dagster-dbt/dagster_dbt/utils.py looks correct to me. Checking invocation.is_successful() after consuming _stream_stdout() addresses the actual bug and should surface the underlying dbt CLI failure instead of letting it turn into a downstream StopIteration/RuntimeError.

One small thing I’d ask about is the unrelated change in python_modules/libraries/dagster-dbt/dagster_dbt/asset_utils.py. I don’t think it introduces a regression, since the old code already failed on empty metadata_by_key, but the new error message is a bit misleading for valid @dbt_assets defs whose selection yields no asset specs, for example test-only selections. It may be worth either:

• tightening the message to mention that the selection may have matched only non-asset dbt nodes, or
• splitting that change into a separate PR since it’s not directly part of the CLI failure fix.

The tests in python_modules/libraries/dagster-dbt/dagster_dbt_tests/core/test_utils.py look adequate for the bug being fixed. I don’t see a blocker here.

[vidiyala99]

Thanks for the review @cmpadden! You're right — the asset_utils.py change was addressing the
symptom (the StopIteration crash) rather than the root cause. With the utils.py fix in place, dbt
CLI failures are surfaced immediately in _select_unique_ids_from_cli, so
get_manifest_and_translator_from_dbt_assets never receives empty metadata from this codepath. The
defensive guard was redundant, and the error message wasn't accurate for the cases where it
would actually fire (e.g., test-only selections).

I've removed the asset_utils.py change — the PR now only contains the utils.py fix and the
corresponding tests.

[cmpadden]

Thanks for the fix here. The production change in dagster_dbt/utils.py looks reasonable, but I don’t think this is ready to land yet because the new tests are patching the wrong symbol.

_select_unique_ids_from_cli imports DbtCliResource inside the function from dagster_dbt.core.resource, while the tests patch dagster_dbt.utils.DbtCliResource. That means the added coverage won’t actually intercept the object being instantiated.

Recommended change: update the tests to patch dagster_dbt.core.resource.DbtCliResource, or move the import in utils.py to module scope and patch that module-level symbol consistently. Once that’s fixed, this looks close.

[cmpadden]

See comment. Thank you.

[vidiyala99]

Updated the tests to patch dagster_dbt.core.resource.DbtCliResource, which matches the
function-local import used by _select_unique_ids_from_cli. Pushed in a50c8a9c12.

[vidiyala99]

@greptile