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

New functionality - Retrieve the description for identical column names from upstream models #61

Merged
merged 7 commits into from
May 17, 2022

Conversation

b-per
Copy link
Contributor

@b-per b-per commented Mar 30, 2022

This is a:

  • bug fix PR with no breaking changes — please ensure the base branch is main
  • new functionality — please ensure the base branch is the latest dev/ branch
  • a breaking change — please ensure the base branch is the latest dev/ branch

Description & motivation

When using the macro generate_model_yaml it can be cumbersome to have to copy/paste the description of the columns from the upstream models, especially when many models are joined together.

The new feature accessible via the flag upstream_descriptions=True (defaulted to False) parses the Jinja graph to retrieve existing descriptions in parent models.

An example of output is the following, when the upstream model has an existing description of description column a:
image

I am not too sure where I should add the entry in CHANGELOG.md

Checklist

  • I have verified that these changes work locally
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my models (and macros if applicable)
  • I have added an entry to CHANGELOG.md

@dbeatty10 dbeatty10 changed the base branch from dev/0.4.0 to main May 13, 2022 12:57
@dbeatty10
Copy link
Contributor

Thanks for this awesome enhancement, @b-per! 💪

Integration test and readme looks great.

Two things for you to do:

  1. I left feedback on two very small things
  2. Example for adding CHANGELOG entry below

Changlog entry

I am not too sure where I should add the entry in CHANGELOG.md

You can follow this pattern for adding to the changelog. Something like this:

# Unreleased
## New features
- Awesome description of fabulous new feature here ([#61](https://github.com/dbt-labs/dbt-utils/pull/61)) 

@dbeatty10 dbeatty10 self-requested a review May 13, 2022 13:18
macros/helpers/helpers.sql Outdated Show resolved Hide resolved
macros/helpers/helpers.sql Outdated Show resolved Hide resolved
dbeatty10
dbeatty10 previously approved these changes May 13, 2022
Copy link
Contributor

@dbeatty10 dbeatty10 left a comment

Choose a reason for hiding this comment

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

Looking good!

Could you take a look at the build in CircleCI? Looks like test_generate_model_yaml_upstream_descriptions isn't passing.

@dbeatty10 dbeatty10 self-requested a review May 13, 2022 15:16
@dbeatty10 dbeatty10 dismissed their stale review May 13, 2022 15:17

Accidentally overlooked CircleCI failure originally

@b-per
Copy link
Contributor Author

b-per commented May 16, 2022

I got quite puzzled about why the test was passing locally but not in CI, before I realized that I needed to add a dbt run to the CI step to get codegen to list the column of the downstream model.

It should be all-good now!

@dbeatty10 dbeatty10 removed their request for review May 17, 2022 12:09
Copy link
Contributor

@dbeatty10 dbeatty10 left a comment

Choose a reason for hiding this comment

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

Nice catch, @b-per

👍 Everything looks good.

@dbeatty10 dbeatty10 merged commit 16727a3 into main May 17, 2022
jeremyholtzman pushed a commit that referenced this pull request Apr 10, 2023
…es from upstream models (#61)

* Update gitignore for latest version of dbt

* Add macros to retrieve info from graph

* Update README with new parameter  for generate_model_yaml

* Add integration test for the new feature

* Update CHANGELOG with upstream_descriptions flag

* Fix typos

* Update CI steps to run the models before testing
@gwenwindflower gwenwindflower deleted the feature/retrieve-parent-column-descriptions branch February 28, 2024 23:01
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.

2 participants