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

0.17.1: opt-in native env rendering #265

Merged
merged 7 commits into from
Jul 9, 2020
Merged

Conversation

jtcohen6
Copy link
Collaborator

@jtcohen6 jtcohen6 commented Jul 7, 2020

Description & motivation

To do before merge

Confirm that the pseudo-code I wrote as examples actually works

Update: All the example code is working when I've checked out dbt-labs/dbt-core#2618. One catch: the freshness example for is_native works without the is_native filter. I'm guessing that the coercion-to-dict is already happening because of hologram. Can you think of a better example?

@jtcohen6 jtcohen6 requested a review from drewbanin July 7, 2020 21:57
@jtcohen6 jtcohen6 marked this pull request as ready for review July 7, 2020 21:57
@clrcrl
Copy link
Contributor

clrcrl commented Jul 8, 2020

Can you also add these to dbt-jinja-functions (page).
In an ideal world they would not need to be updated in two places, but, alas!

Copy link
Collaborator

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

Couple of comments here but all minor - looks good to me once those are addressed, but of course let me know if you disagree with any of them!

website/docs/reference/dbt-jinja-functions/as_bool.md Outdated Show resolved Hide resolved
website/docs/reference/dbt-jinja-functions/as_text.md Outdated Show resolved Hide resolved
@jtcohen6
Copy link
Collaborator Author

jtcohen6 commented Jul 8, 2020

@drewbanin Do you have any ideas for a better is_native example? A case where it's actually/uniquely needed

@drewbanin
Copy link
Collaborator

I can't think of a good use case where we'd want to recommend as_native over as_number or as_bool. Maybe the one use case is around returning tuple/list/dict/set values from a jinja expression? I think that's pretty uncommon....

@jtcohen6
Copy link
Collaborator Author

jtcohen6 commented Jul 9, 2020

I spent too long trying to come up with a sane example where is_native is strictly necessary. In the end, I just removed its Usage section and added a note recommending as_bool or as_number instead.

@jtcohen6 jtcohen6 merged commit 45f7e71 into master Jul 9, 2020
@jtcohen6 jtcohen6 deleted the feature/native-env-chg-0-17-1 branch July 9, 2020 00:07
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