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

feat: allow column description to be overwritten #291

Merged
merged 8 commits into from
Nov 27, 2024

Conversation

sabino
Copy link
Contributor

@sabino sabino commented Nov 19, 2024

This allows description to be set using metabase.description at column and table level, prioritizing it when set.

Feel free to suggest any possible changes, but just to give you an idea of our use case. We might (and need) different descriptions for columns/tables due to: language, size (dbt documentation allows for more flexibility).

I might update the PR to include the same behavior at Table level. Done.

@gouline
Copy link
Owner

gouline commented Nov 19, 2024

Yeah, if you're adding it for columns, it should be there for tables too. Add some unit tests as well.

@sabino
Copy link
Contributor Author

sabino commented Nov 26, 2024

I've added it to the Model and updated the tests. WDYT?

Copy link
Owner

@gouline gouline left a comment

Choose a reason for hiding this comment

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

CI is failing, make check test should succeed locally before pushing any commits

tests/fixtures/manifest-v12.json Outdated Show resolved Hide resolved
dbtmetabase/manifest.py Outdated Show resolved Hide resolved
dbtmetabase/manifest.py Outdated Show resolved Hide resolved
dbtmetabase/manifest.py Outdated Show resolved Hide resolved
dbtmetabase/manifest.py Outdated Show resolved Hide resolved
dbtmetabase/manifest.py Outdated Show resolved Hide resolved
dbtmetabase/manifest.py Outdated Show resolved Hide resolved
dbtmetabase/manifest.py Outdated Show resolved Hide resolved
dbtmetabase/manifest.py Outdated Show resolved Hide resolved
@gouline gouline changed the base branch from master to meta-description November 27, 2024 03:29
@gouline gouline merged commit 9c1ea40 into gouline:meta-description Nov 27, 2024
1 check failed
@gouline gouline mentioned this pull request Nov 27, 2024
gouline added a commit that referenced this pull request Nov 27, 2024
* feat: allow column description to be overwritten (#291)

* feat: allow column description to be overwritten

* feat: Consider Model description as well

* chore: Update tests

* chore: Column test

* fix: fmt

* fix: test v2 manifest instead of v12

* Apply suggestions from code review

---------

Co-authored-by: Mike Gouline <[email protected]>

* fix type issue

---------

Co-authored-by: Felipe Guilherme Sabino <[email protected]>
@sabino sabino deleted the feat/column-description branch November 27, 2024 12:16
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