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

Overwriting rendered column descriptions with the unrendered yaml #129

Merged
merged 18 commits into from
Mar 28, 2024

Conversation

VDFaller
Copy link
Contributor

@VDFaller VDFaller commented Feb 20, 2024

Fixes #128

Obviously this isn't the final but I got this working if I move the docs block from orders to stg_orders. in the demo_sqlite jaffle_shop. Still unclear what patch_path really is so don't know if that's a great idea.

Just making sure I'm not barking up the wrong tree.

Currently looking for a function in dbt core to just get the relevant dict from the fqn or unique_id or something like that.

@z3z1ma
Copy link
Owner

z3z1ma commented Mar 12, 2024

@VDFaller patch_path is what dbt for years has referred to the the YAML metadata as on the inside of its lib. They call it that because they generate nodes prior to "patching" in the yaml metadata AFAIK. So totally the right thing to use to find the complementary YAML for a model/seed node.

My only thought here is to make sure we know for certain what dbt versions the api your consuming is compatible with so we can advertise it to users. But directionally this looks right to me and that function that constructs the dict with the description key is exactly where id put this.

@z3z1ma
Copy link
Owner

z3z1ma commented Mar 12, 2024

LMK if you are comfortable with the click part, otherwise I can help run this to the finish line. Up to you.

@VDFaller
Copy link
Contributor Author

@z3z1ma is your testing setup for multiple dbt versions it you just want me to manually test? Or just look at manifest versions ?

I'll take a stab at click, been wanting to learn it anyways.

@VDFaller
Copy link
Contributor Author

@z3z1ma I moved the doc string to raw, so I can make a test. But the below doesn't work. The click is going through so maybe I don't have my environment setup correctly?

cd demo_duckdb
dbt-osmosis yaml document orders --use-direct-yaml-descriptions

Am I missing something? As knowledge and whatnot looks right to me. But when I look at status in the schema file it just adds data type with no new description.

      - name: status
        tests:
          - accepted_values:
              values: ['placed', 'shipped', 'completed', 'return_pending', 'returned']
        data_type: string

@z3z1ma
Copy link
Owner

z3z1ma commented Mar 13, 2024

I can try it on my environment to see if I can sniff anything out. Looks right AFAICT.

@z3z1ma
Copy link
Owner

z3z1ma commented Mar 20, 2024

This section
https://github.com/VDFaller/dbt-osmosis/blob/inherit-from-yaml/src/dbt_osmosis/core/column_level_knowledge.py#L28-L32
Needs to be updated to include seeds
so k["progenitor"].startswith("source")
should be (k["progenitor"].startswith("source") or k["progenitor"].startswith("seed")) since a seed is effectively a source, meaning its always a base node

I added that and validated the the description was propagated successfully as a doc block @VDFaller

Copy link
Owner

@z3z1ma z3z1ma left a comment

Choose a reason for hiding this comment

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

Just a useful tidbit. If you are in the demo_duckdb directory, you can do dbt build -t test and it works out of the box. Just delete the existing .duckdb file if its there. After that you can dbt-osmosis yaml refactor/document/organize -t test [other args]

src/dbt_osmosis/core/column_level_knowledge_propagator.py Outdated Show resolved Hide resolved
src/dbt_osmosis/core/column_level_knowledge_propagator.py Outdated Show resolved Hide resolved
src/dbt_osmosis/main.py Outdated Show resolved Hide resolved
Copy link
Owner

@z3z1ma z3z1ma left a comment

Choose a reason for hiding this comment

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

LGTM, as I said in our slack chain -- fantastic work

@z3z1ma
Copy link
Owner

z3z1ma commented Mar 22, 2024

Ill handle documentation update and version bump for pypi

@VDFaller
Copy link
Contributor Author

Sorry I never got back to you. yeah I'm all good.

@z3z1ma z3z1ma merged commit 1c16714 into z3z1ma:main Mar 28, 2024
2 checks passed
@VDFaller VDFaller deleted the inherit-from-yaml branch May 14, 2024 22:59
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.

Allow docs block descriptions to propagate un rendered.
2 participants