dbt-labs / dbt-snowflake

dbt-snowflake contains all of the code enabling dbt to work with Snowflake
https://getdbt.com
Apache License 2.0
297 stars 177 forks source link

[Bug] dynamic tables: support macros in "snowflake_warehouse" config value #1097

Open mike-weinberg opened 5 months ago

mike-weinberg commented 5 months ago

Is this your first time submitting a feature request?

Describe the feature

in most materialization types in snowflake, i can use a macro for dynamically switching what warehouse is in use based on if i am in dev or prod. it seems that when i try this in a DT materialization, I get an error.

eg:

{{
config(
    materialized = 'dynamic_table',
    snowflake_warehouse = 'my_prod_xsmall_warehouse',
    target_lag = 'downstream',
    on_configuration_change = 'apply',
)
}}

works fine.

{{
config(
    materialized = 'dynamic_table',
    snowflake_warehouse = get_warehouse('xsmall'),
    target_lag = 'downstream',
    on_configuration_change = 'apply',
)
}}

yields an error like the following sanitized error:

20:03:57 SQL status: SUCCESS 1 in 1.0 seconds
20:03:57 On model.EnergyHub.my_dynamic_table: Close
20:03:57 Compilation Error in model my_dynamic_table (models/.../my_dynamic_table.sql)
  SnowflakeDynamicTableConfig.__init__() missing 1 required positional argument: 'snowflake_warehouse'

  > in macro dynamic_table_get_build_sql (macros/materializations/dynamic_table.sql)
  > called by macro materialization_dynamic_table_snowflake (macros/materializations/dynamic_table.sql)
  > called by model my_dynamic_table (models/.../monitor/my_dynamic_table.sql)
20:03:57 1 of 1 ERROR creating sql dynamic_table model DEV_DB.SCHEMA.my_dynamic_table  [ERROR in 1.23s]
20:03:57 Finished running node model.PROJECT.my_dynamic_table
20:03:57   Compilation Error in model my_dynamic_table (models/.../my_dynamic_table.sql)
  SnowflakeDynamicTableConfig.__init__() missing 1 required positional argument: 'snowflake_warehouse'

  > in macro dynamic_table_get_build_sql (macros/materializations/dynamic_table.sql)
  > called by macro materialization_dynamic_table_snowflake (macros/materializations/dynamic_table.sql)
  > called by model my_dynamic_table (models/.../my_dynamic_table.sql)

unfortunately i cannot figure out exactly what the problem is, but it is notable that this config comes from the extra part of the dbt_adapter repo's contract in relation.py

I assume this has something to do with the parser but tbh that's all black magic to me.

Describe alternatives you've considered

No response

Who will this benefit?

dynamic table users for whom their company requires separation of warehouses between production and dev and use a macro in order to dispatch based on what environment they're in.

Are you interested in contributing this feature?

happily, i just need direction =))))

Anything else?

No response

dataders commented 4 months ago

I've asked the engineering team to take a look. Here's some shots in the dark in the interim.

SnowflakeDynamicTableConfig.init() missing 1 required positional argument: 'snowflake_warehouse'

questions

  1. can you find which line within dynamic_table_get_build_sql causes the error? the closer you can get us to the error, the easier to solve.
  2. does adding a a -d debug flag (dbt -d run ...) give you any more information?
  3. does the same error happen when it's a brand new DT vs. one that's being modified?
mike-weinberg commented 4 months ago

WOO thank you i can take a look at some of these, much appreciated @dataders !

mike-weinberg commented 4 months ago

hi! ok little status update:

  1. changing the config change parameter to "fail" made no difference. no real surprise there.
  2. using the debug flag doesn't seem to make a difference. I'm using the dbt cloud ide, so maybe that makes a difference?
  3. python ternaries work just fine: 'developer__small' if not target=='prod' else '__svc__prod__transformations__small' works like a charm (and is a viable albeit tedious workaround)
  4. moving the ternary to an external jinja assignment and referencing the jinja-defined variable works!
  5. moving the macro call to a variable assignment does not work. eg {% set warehouse = get_warehouse('small') %} and then referencing that in the config, yields the same error.
  6. i tried locally defining the macro, but it seems that isn't supported per this issue, so i can't test whether the nonlocality of the macro (and the likely possibility that macros aren't parsed before the extra aspect of configs are parsed)
  7. building a dynamic table where none already exists does not appear to change anything.

so the things I know are:

  1. simple inline python expressions work
  2. jinja in the same file works
  3. jinja defined in a macro in another file doesn't work
  4. changing the on-configuration-change doesn't seem to matter
  5. the configuration for snowflake warehouse for dynamic tables comes from a place in the codebase which is different from the part of the codebase used by other materializations to get the snowflake warehouse

so from all of these things I suspect that this has to do with differences in how the extra config is used for snowflake dynamic tables which differs from how this is used for other materialization types, namely grabbing it from the top level of a standard config, which appears (after digging) to be part of the BaseAdapter class as part of its class init method.

I might be way way off base here but it seems like these differences in how snowflake warehouse is initialized results in a difference in how they are parsed, such that one can use macros and the other cannot.

I wondered if this is a partial parsing issue, so i tried running a dbt clean first. that made no difference.

So unless I drop out of dbt cloud and start mucking with dbt itself, I am a little stuck. If you have suggestions for how to develop on DBT itself, I'd love to do some sort of intro session to developing on dbt.

colin-rogers-dbt commented 4 months ago

@mike-weinberg taking a look into this, what dbt version are you using?

apd-bbaker commented 1 month ago

@colin-rogers-dbt Running into this as well on v1.8.3