InfuseAI / piperider

Code review for data in dbt
https://www.piperider.io/
Apache License 2.0
480 stars 23 forks source link

[Bug] Handle naming conflict problem in profiler and report-generator #808

Closed qrtt1 closed 1 year ago

qrtt1 commented 1 year ago

PR checklist

What type of PR is this?

bugfix

What this PR does / why we need it:

In a real case, users might use the same name for different dbt nodes.

For example:

(venv) (⎈ |kind-kind:default)(base) ➜ tw_campaign_finance git:(chore/rename) ✗ dbt build 03:17:51 Running with dbt=1.5.2 03:17:51 Registered adapter: duckdb=1.5.2 03:17:51 Encountered an error: Compilation Error dbt found two models with the name "transaction".

Since these resources have the same name, dbt will be unable to find the correct resource when looking for ref("transaction").

To fix this, change the name of one of these resources:

  • model.tw_campaign_finance.transaction (models/staging/transaction.sql)
  • model.tw_campaign_finance.transaction (models/transaction.sql)

Which issue(s) this PR fixes:

sc-31825

To Reviewers:

There are two parts in the main goal


Ref data for broken report

(venv) (⎈ |kind-kind:default)(base) ➜  tw_campaign_finance git:(chore/rename) ✗ g show
commit eece695a3924161e959e0718096e3ed6ca6dcdd0 (HEAD -> chore/rename)
Author: Ching Yi, Chan <qrtt1@infuseai.io>
Date:   Wed Jul 26 19:18:57 2023 +0800

    rename to transaction

    Signed-off-by: Ching Yi, Chan <qrtt1@infuseai.io>

diff --git a/models/corporation_contribution.sql b/models/corporation_contribution.sql
index baea214..f116b66 100644
--- a/models/corporation_contribution.sql
+++ b/models/corporation_contribution.sql
@@ -32,7 +32,7 @@ select
     industry,
     industry_category

-from {{ ref('campaign_transaction') }}
+from {{ ref('transaction') }}
     left join corp_industry_category on
         who_id = corp_industry_category.id

diff --git a/models/campaign_transaction.sql b/models/transaction.sql
similarity index 100%
rename from models/campaign_transaction.sql
rename to models/transaction.sql

Comparison Summary

Resource Total Impacted Explicit Changes Implicit Changes
Models 2 3 3 0
Metrics 0 0 0 0

Models

Model Columns
Rows Dbt Time Failed Tests All Tests
models/transaction.sql None nan 0:00:03.11 - -
..els/campaign_transaction.sql 26 nan
..corporation_contribution.sql 9 ($\color{green}{\text{ 0 }}$ / $\color{red}{\text{ 0 }}$ / $\color{orange}{\text{ 0 }}$) nan 0:00:00.15 $\color{green}{\text{ (↓ -0.01) }}$ - -

Metrics

No changes detected

codecov[bot] commented 1 year ago

Codecov Report

Merging #808 (78b4b04) into main (49bbe76) will increase coverage by 0.06%. The diff coverage is 60.86%.

@@            Coverage Diff             @@
##             main     #808      +/-   ##
==========================================
+ Coverage   36.34%   36.41%   +0.06%     
==========================================
  Files          68       68              
  Lines        9844     9879      +35     
==========================================
+ Hits         3578     3597      +19     
- Misses       6266     6282      +16     
Files Changed Coverage Δ
piperider_cli/cloud/__init__.py 0.00% <0.00%> (ø)
piperider_cli/runner.py 20.27% <0.00%> (-0.15%) :arrow_down:
piperider_cli/profiler/profiler.py 74.12% <66.66%> (-0.40%) :arrow_down:
piperider_cli/dbt/utils.py 91.09% <85.00%> (-1.13%) :arrow_down:
piperider_cli/dbt/changeset.py 90.96% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

qrtt1 commented 1 year ago

Sample for the new profiling data:

image

sample.json.txt

qrtt1 commented 1 year ago

Found the output can not pass the schema validator:

image
qrtt1 commented 1 year ago

Broken data has been fixed.