[CT-109] [Feature] Show a hint in the error message when trying to access the graph whenever execute = False

[joellabes]

Is there an existing feature request for this?

• I have searched the existing issues

Describe the Feature

Today in "Joel wants better error messages": execute!

This is a common tripwire - I trip over it myself about once a year.

Is there anything we can do when someone tries to do something that works in execute mode, but not during the first runthrough? The first thread I linked above was particularly insidious because when I tried to debug it by outputting inside of {{ curly braces }} it worked correctly

Describe alternatives you've considered

No response

Who will this benefit?

People writing their own introspective queries, or code that depends on the graph object (likely to become more popular, especially to access metric definitions etc)

Are you interested in contributing this feature?

No response

Anything else?

No response

[joellabes]

I'm closing out comments on the metrics package's PR and didn't want this to get lost:

jtcohen6 yesterday

Re: this, and this, and #4641: I think we might just want to add some new context-member methods like:

get_metric()
get_exposure()
get_node()

Crucially, they'd:

take the node name (or unique_id?) as an argument, and all they'd do is return the manifest entry for that node/metric/exposure, so you don't need any graph.things.values() looping or selectattr cleverness
raise an error or return nothing at parse time (execute = False)

(cc @emmyoop — I see you assigned yourself to 4641, and we were just talking about context methods earlier)

[jtcohen6]

Some wild-eyed experimentation with what a get_metric() contextmember might look like: bd6a4a1

There are two main goals here:

• Make it possible to access metric information without introspecting the graph
• Make it possible to query the metric's model, without needing to pass it into a nested/unparsed ref()

I'm going to open a PR against the dbt_metrics package, for a quick vibe check of the code cleanup it makes possible: dbt-labs/dbt_metrics#11

This also helped me get to the bottom of some weird issues that @joellabes and I had been seeing months ago, whereby a nested/unparsed ref() would sometimes raise an error ("dbt was unable to infer all dependencies..."), sometimes not. Turns out that we've always had different logic for what to do with a previously unknown ref, depending on the runtime context:

• RuntimeRefResolver (standard) will raise a bad_ref_context error: "dbt was unable to infer all dependencies..."
• By contrast, OperationRefResolver simply does not care

It raises an interesting dilemma:

• If you're querying a metric as a macro/operation, it just works!
• If you're including a metric calculation in a model... should you be? Is that what metrics are for? :)

You can actually get it working by explicitly specifying the underlying model reference:

-- models/my_model.sql

-- depends on: {{ ref('dim_customers') }}
select * 
from {{ metrics.metric(
    metric_name='new_customers',
    grain='week',
    dimensions=['plan', 'country']
) }}

Does that even feel so wrong? The DAG lineage kinda works, doesn't it? Or should we make it possible for a model to ref a metric for real? (We've already talked some about metrics ref'ing metrics, e.g. to calculate ratios)

Note: Still need to confirm the behavior when running via new dbt-server. If that requires a manually commented ref(), it's no good, and we should find another way.

[joellabes]

If you're including a metric calculation in a model [...] you can actually get it working by explicitly specifying the underlying model reference. Does that even feel so wrong?

I think it's reasonable for now! Bonus: by it being a bit of a pain in the butt, it may help to discourage people from assuming it's the default way to use the metrics package 😂

In the medium term, yeah I think we'll need to be able to calculate metrics on top of one another, but I don't know that it has to be materialised as a model in the middle. (Although given how grumpy Redshift gets when you nest too many CTEs deep, it might be important to have a table as a stabilising point in the middle).

[jtcohen6]

Note: Still need to confirm the behavior when running via new dbt-server. If that requires a manually commented ref(), it's no good, and we should find another way.

I haven't been able to confirm this by testing locally, but by reading through the codebase, I'm pretty sure this would be an issue. I've also convinced myself that it's a good thing the manual ref() is required in models (DAG nodes). So the question is just, how to avoid raising this error when using the compile_sql + execute_sql methods on a bundle of arbitrary SQL, i.e. a metrics query.

To that end, I've opened #4851. We'll need to make that change (or one like it) before we can implement the good version of get_metric.

[jtcohen6]

The motivating ask here (metrics) will be resolved by #5027. Even though that doesn't resolve the initial ask per se—raising an error if graph is accessed at parse time—I think it's less likely to come up now that we don't need as many hijinks in the metrics package. Closing for now.