Datavault-UK / automate-dv

A free to use dbt package for creating and loading Data Vault 2.0 compliant Data Warehouses (powered by dbt, an open source data engineering tool, registered trademark of dbt Labs)
https://www.automate-dv.com
Apache License 2.0
478 stars 114 forks source link

[BUG] src_payload expansion missing from multi-active satellite macro #217

Closed jackburtone closed 4 months ago

jackburtone commented 8 months ago

Describe the bug I think there is some code missing from the tables/sqlserver/masat.sql

Environment dbt version: 1.6.3 automate_dv version: 0.10.1

Database/Platform:
Databricks

To Reproduce Steps to reproduce the behavior:

  1. Create a sateliite model
  2. add the automatedv.masat() call to the macro
  3. run the model
  4. See error "Invalid parameters provided to prefix macro. Expected: (columns [list/string], prefix_str [string]) got: (, s)

    in macro defaultprefix (macros/supporting/prefix.sql) called by macro prefix (macros/supporting/prefix.sql) called by macro defaultprefix (macros/supporting/prefix.sql) called by macro prefix (macros/supporting/prefix.sql) called by macro defaultprefix (macros/supporting/prefix.sql) called by macro prefix (macros/supporting/prefix.sql) called by macro sqlserverma_sat (macros/tables/sqlserver/ma_sat.sql) called by macro databricks__ma_sat (macros/tables/databricks/ma_sat.sql) called by macro ma_sat (macros/tables/snowflake/ma_sat.sql)"

Expected behavior expect the macro to run

Screenshots If applicable, add screenshots to help explain your problem.

Log files If applicable, provide dbt log files which include the problem.

Additional context I did a local fix by overriding the default multiactive macro and adding this line. So I think it needs to be added to the default code.

{%- macro default__ma_sat(src_pk, src_cdk, src_hashdiff, src_payload, src_extra_columns, src_eff, src_ldts, src_source, source_model) -%} {# added missing line -#} {%- set src_payload = automate_dv.process_payload_column_excludes( src_pk=src_pk, src_hashdiff=src_hashdiff, src_payload=src_payload, src_extra_columns=src_extra_columns, src_eff=src_eff, src_ldts=src_ldts, src_source=src_source, source_model=source_model) -%}

DVAlexHiggs commented 8 months ago

Thanks for this report. All of our tests are running and passing so it might be something on your end.

Can you show us your model with metadata (modified for privacy if needed) please?

Please also see the docs for example metadata and ensure your metadata structure matches: https://automate-dv.readthedocs.io/en/latest/metadata/#multi-active-satellites-mas

Thanks!

jackburtone commented 8 months ago

Hi there, to clarify it happens when you set the payload to do excludes rather than listing all the columns. source model code (modified for privacy) is :


{{ config(materialized="incremental", tags=["sat", "da"]) }}

{%- set yaml_metadata -%}
source_model: "staging_customer_address"
src_pk: "customer_hk"
src_cdk:
  - "addressid"
src_hashdiff: "address_hashdiff"
src_payload:
  exclude_columns: true
  columns:
    - "ingestion_date"
src_eff: "effective_from"
src_ldts: "load_datetime"
src_source: "source"
{%- endset -%}

{%- set metadata_dict = fromyaml(yaml_metadata) -%}

{{ automate_dv.ma_sat(
  source_model=metadata_dict["source_model"],
   src_cdk=metadata_dict["src_cdk"],
  src_pk=metadata_dict["src_pk"],
  src_hashdiff=metadata_dict["src_hashdiff"],
  src_payload=metadata_dict["src_payload"],
  src_eff=metadata_dict["src_eff"],
  src_ldts=metadata_dict["src_ldts"],
  src_source=metadata_dict["src_source"]
) }}```
DVAlexHiggs commented 8 months ago

Ah, I see now. Apologies for missing this in your original post. MAS doesn't officially support the payload exclude feature - however as you have found it should be as simple as adding this line.

We can add this in the next release as we want to make sure these features are consistent throughout the package.

Thank you!

jackburtone commented 8 months ago

OK great thank you! We will keep our fix in place til that release then.

DVAlexHiggs commented 4 months ago

Fixed in v0.10.2 😄 Thanks for your patience for release of this! Please let us know if you experience any issues by responding here or opening a new issue.