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: support dbt model versions #205

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

henriblancke
Copy link
Contributor

Hi, I'm adding support for dbt model versions. We rely on versioning heavily and would love for dbt-osmosis to support it. Thanks 🙏

@z3z1ma
Copy link
Owner

z3z1ma commented Jan 24, 2025

Looks great, really nice and thx for the contribution!

I suspect we will need to update this too
https://github.com/z3z1ma/dbt-osmosis/blob/v1.1.9/src/dbt_osmosis/core/osmosis.py#L1689
If we want the unrendered version of information to be usable such as {{ docs() }} in versioned models, otherwise we will inadvertently hydrate them.

In fact, looking closer _get_node_yaml (the function above) should probably take a kwarg that determines if its read-write or read-only
And that function should be the sole place where we grab a mutable "slice" of the deserialized data object. This would make our code more DRY in one of the last spots.
Its not incumbent on you to do unless your bored or really feeling it :)
Otherwise I will try to get to it this week and we can merge this as-is. LMK either way. I'll just want the above tackled before I cut a patch release.

@henriblancke
Copy link
Contributor Author

Hi @z3z1ma thanks for the quick feedback. I was hoping to have more time this week to tackle your suggestion but it looks like its shaping up to be a busy week. I can probably take a closer look either during the weekend or next week 🤞. Maybe we can tackle it as part of a different PR? Thanks again!

@z3z1ma
Copy link
Owner

z3z1ma commented Jan 29, 2025

Sure thing, I'll merge it now. Thanks again.

@z3z1ma z3z1ma merged commit 23b4519 into z3z1ma:main Jan 29, 2025
7 checks passed
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