Skip to content
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

Add docs a for exposure generic tests #1154

Merged
merged 3 commits into from
Sep 28, 2023

Conversation

erikzaadi
Copy link
Contributor

Depends on elementary-data/dbt-data-reliability#530

Added docs for the new exposure validation tests:

image image image

@erikzaadi erikzaadi requested a review from ellakz September 11, 2023 14:34
@linear
Copy link

linear bot commented Sep 11, 2023

ELE-1703 Create generic test for exposure stability

Definition of done: Create a generic test defined per model. The test iterates on the defined exposures and looks for exposures that depend on the model, and checks the columns described in the exposure match the columns in the model (name and data type).

@github-actions
Copy link
Contributor

👋 @erikzaadi
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

@erikzaadi erikzaadi changed the title Add docs a for exposure generic tests [WIP] Add docs a for exposure generic tests Sep 11, 2023
## Exposure validation dbt tests

Elementary dbt package includes **exposure validation tests, implemented as [dbt tests](https://docs.getdbt.com/docs/building-a-dbt-project/tests)**.
These tests can detect missing columns and column data types for defined exposures.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikzaadi
This is a technical description of the test.
I think it's better to describe here the use case it solves.
"These tests will fail if there are column changes that break downstream exposures, such as BI dashboards."

Also, it's a good place to say that Elementary Cloud automatically generates exposures, maps the lineage, and adds the test to the models that the exposures depend on.

@erikzaadi erikzaadi force-pushed the ele-1703-create-generic-test-for-exposure-stability branch from 83deb76 to a098d3c Compare September 11, 2023 17:58

```

We recommend adding a tag to the tests so you could execute these in a dedicated run using the selection parameter `--select tag:elementary`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have this recommendation in all the test types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs/guides/add-exposure-tests.mdx Outdated Show resolved Hide resolved
data_type: "numeric"
```

Then in your module schema, add the elementary exposure tests:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to point out that the test needs to be set up for every model used in the exposure.
Also you might wanna explain a little on how the test works, mainly to communicate why no other setup is needed except this test :)

docs/guides/add-exposure-tests.mdx Outdated Show resolved Hide resolved
source: ref('returned_orders')
- name: "customer_id"
data_type: "numeric"
source: ref('orders')
Copy link
Contributor

@ellakz ellakz Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's possible, but it will be nice if we could do something like:

referenced_columns:
  ref('orders'):
    - name: "order_id"
      data_type: "numeric"
    - name: "customer_id"
      data_type: "numeric"
  ref('customers'):
    - ... 

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure, and it'll make the "simple" workflow (AKA no need to set the source property since there's only one exposure dependency) obsolute, which IMHO is a shame, it's prolly our 80 vs 20 scenario

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep I agree. but maybe we can rename "name" and "source" to "column_name" and "model_name"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed, only difference is node instead of model_name as per our conversation and @haritamar 's comment on the main PR

docs/guides/add-exposure-tests.mdx Outdated Show resolved Hide resolved
@erikzaadi erikzaadi force-pushed the ele-1703-create-generic-test-for-exposure-stability branch 2 times, most recently from c32c7fd to 579ed26 Compare September 20, 2023 07:44
@erikzaadi erikzaadi marked this pull request as ready for review September 20, 2023 07:48
@erikzaadi erikzaadi changed the title [WIP] Add docs a for exposure generic tests Add docs a for exposure generic tests Sep 20, 2023
@erikzaadi erikzaadi force-pushed the ele-1703-create-generic-test-for-exposure-stability branch 2 times, most recently from 5aca7de to 2e00e6c Compare September 26, 2023 17:30
@ellakz ellakz self-requested a review September 27, 2023 07:59
@erikzaadi erikzaadi force-pushed the ele-1703-create-generic-test-for-exposure-stability branch from 2e00e6c to a366ba4 Compare September 28, 2023 13:41
@erikzaadi erikzaadi merged commit a366ba4 into master Sep 28, 2023
2 checks passed
@erikzaadi erikzaadi deleted the ele-1703-create-generic-test-for-exposure-stability branch September 28, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants