@rex finally getting a chance to play around with the new

DbtCli

. I created an interesting scenario that doesn't make sense to me. dbt is erroring on a model, but the asset materialization is still successful (no assets are yielded at least, which is correct for an error). Assets from

load_assets_from_dbt_manifest

did not behave this way. Does this behavior need to be setup explicitly now? I don't see the new resource ever raising

DagsterDbtCliHandledRuntimeError

or a new equivalent.

GitHub is down right now, so I can’t link a similar discussion in the RFC. But yes overall: • This behavior is now up to the user to decide what should happen: you must handle what should happen in an error or success case. The underlying

DbtCliTask

that’s created by

DbtCli.cli(...)

has this information from

is_successful()

• And we essentially defer behavior to whatever happens in the underlying CLI process here: https://docs.getdbt.com/reference/exit-codes • The reason why we did this was we didn’t want to implicitly assume what should happen in this error scenario, especially when there are already levers to customize this behavior (e.g.

--fail-fast

(link),

--warn-error-options

(link)).

It did indeed. Sorry for the thinking aloud!

cli_task = dbt.cli(
            args,
            manifest=manifest,
            context=context,
        )
        yield from cli_task.stream()

        if not cli_task.is_successful():
            raise Exception("dbt command failed")

no need to apologize — this is a change in behavior and you are right to be surprised

I have the tab open (thankfully) - must have just skimmed too quickly to miss the related discussion

Looks like we’re back! Here’s the previous discussion on it if you are curious: https://github.com/dagster-io/dagster/discussions/14477#discussioncomment-6092952

One more difference - is there no longer an equivalent of

capture_logs

? I'm not sure I actually want it compared to just using stdout logs capture - need to mull that over. I suppose it's trivial to build myself via

stream_raw_events

You would rather use the event logs to see your raw dbt logs rather than compute logs?

I don't think I actually want to - just used to that option. I suspect compute logs is much better - no need to put the dbt logs in postgres anyways

Yeah we believe the compute log experience is a lot better here, especially since we are streaming logs. If we streamed the raw dbt logs into the event stream, they would also show up in separate events, which would be a bit illegible. And we if we waited for all the events to come in before displaying an event in the event stream, then it wouldn’t really streaming the logs!

fwiw, when I implemented the new decorator in our demo project, I also wanted to see the original error behavior and the event log output, so I used this pattern: https://github.com/dagster-io/hooli-data-eng-pipelines/blob/master/hooli_data_eng/assets/dbt_assets.py#L77 https://github.com/dagster-io/hooli-data-eng-pipelines/blob/master/hooli_data_eng/assets/dbt_assets.py#L100-L101

@Adam Bloom: we changed the behavior to raise an error on exception, since it looks like this behavior is unintuitive for users. We changed it now so that: • By default, if the underlying dbt CLI process has a non-zero exit code, an error will be raised. In the Dagster UI, the step will show as failed. • You can turn this behavior off by adding

raise_on_error=False

to

.cli(…)

(e.g.

.cli(…, raise_on_error=False)

https://github.com/dagster-io/dagster/pull/15055

thanks for the info @rex. I'll remove the explicit success check when I update dagster-dbt next. In general, I really liked my first foray into the new API! I only used it on the new asset I was adding, but I'm excited to find time to go and overhaul our existing dbt assets to use the new stuff.

love to hear it! let us know if you have other feedback