-
Notifications
You must be signed in to change notification settings - Fork 497
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
union_relations
with exclude
param on unmodified models not working with dbt docs generate
in CI run
#606
Comments
dbt-utils/macros/sql/union.sql Lines 64 to 77 in b4931fe
Looks like we raise the error_message here when Something like this could do the trick... is this right or not... 🤔 {%- if flags.WHICH != 'generate' -%}
{{ exceptions.raise_compiler_error(error_message) }}
{%- endif -%}
{%- endif -%} |
🙌 Thanks for the thorough writeup, I'm the person who originally reported this bug to support! Following the issue for updates, but it seems like if we drop the excludes we can get docs back on CI runs? |
Agreed, incredible investigation and write-up @jeremyyeo 🏅 Following the examples you provided, I was also able to reproduce this outside of dbt Cloud and CI. The short story
Successful solutionYour solution based on Side note: looks like Potential implementationThis particular macro has been the subject of a handful of issues and PRs recently, so we'll want to be thoughtful about the final implementation to avoid an accidental regression. At the same time, it would be nice to not just layer on more patch work in this section of code. Recent history of issues and pulls related to this issue: (#266, #473) and (#505, #509). Seemingly unrelated open issues for Merely doing something like the following might have solved #266: select
cast({{ dbt_utils.string_literal(relation) }} as {{ dbt_utils.type_string() }}) as {{ source_column_name }}{% if ordered_column_names %},{% endif -%} If we want to continue raising errors in certain circumstances, I suspect we could do something like the following: {#-- If only compiling or generating documentation, it's okay if no columns are found -#}
{%- set dbt_command = flags.WHICH -%}
{% if dbt_command in ['run', 'build'] and not column_superset.keys() %}
{%- set relations_string -%}
{%- for relation in relations -%}
{{ relation.name }}
{%- if not loop.last %}, {% endif -%}
{%- endfor -%}
{%- endset -%}
{%- set error_message -%}
There were no columns found to union for relations {{ relations_string }}
{%- endset -%}
{{ exceptions.raise_compiler_error(error_message) }}
{%- endif -%} Ultimately, we might want to do both. Main concernI'm not aware of any tests that cover the original errors, so it will be harder to be confident that we're avoiding any regressions. I think (hope?) we can reason our way through this though. |
Good choice on not using |
Good call-out about Do you know if there's any way to differentiate |
Forgot about the rpc flags but copying a convo between Scott and I the last time we looked at this: Macro like this:
In a hook:
Tested in IDE:
Only The reset should be what their command actually is. Totes weird. |
Describe the bug
Now... that's a long title 😅
Basically if you're using the
exclude
param in theunion_relations
macro in a model that isn't part of your CI run,dbt docs generate
throws an error.Steps to reproduce
Preamble
Make sure we already have a prod run (
dbt build
) and a CI run (dbt run --select state:modified+
) setup in dbt Cloud with the generate docs option ticked.dbt build
prod run and see that there are no issues:baz
model:^ Notice the
Runtime Error
.Expected results
dbt docs generate
to not error.Actual results
dbt docs generate
errors with aRuntime Error
.Screenshots and log output
System information
The contents of your
packages.yml
file:Which database are you using dbt with?
The output of
dbt --version
:Additional context
Initially I thought that the log lines:
contributed to this - but if you actually do not use the
exclude
param - e.g.You still see the same lines logged BUT there is no error emitted. This is an example log of just that happening:
Are you interested in contributing the fix?
Sure - haven't fully digested why yet though.
The text was updated successfully, but these errors were encountered: