Make warn return empty string (#2222)

[jeremyyeo]

resolves #2222

Description

Fixes the compilation of {{ exceptions.warn("thing") }} to return an empty string"" instead of None.

Checklist

• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Jeremy Yeo.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email email@example.com
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[beckjake]

@cla-bot check

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Jeremy Yeo.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email email@example.com
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[jeremyyeo]

@beckjake is it necessary to add a test for this one?

[beckjake]

@jeremyyeo Thanks for your contribution!

Can you add an integration test so we can be sure we don't regress? There's an existing test that uses {% do exceptions.warn(...) %}. Can you update it to also do {{ exceptions.warn(...) }}? The file in question is at dbt/test/integration/013_context_var_tests/emit-warning-models/warning.sql. I don't think you'll have to update the Python at all, it should have the exact same pass/fail characteristics with the second warning.

[beckjake]

@beckjake is it necessary to add a test for this one?

What a timely question :)

[jeremyyeo]

@jeremyyeo Thanks for your contribution!

Can you add an integration test so we can be sure we don't regress? There's an existing test that uses {% do exceptions.warn(...) %}. Can you update it to also do {{ exceptions.warn(...) }}? The file in question is at dbt/test/integration/013_context_var_tests/emit-warning-models/warning.sql. I don't think you'll have to update the Python at all, it should have the exact same pass/fail characteristics with the second warning.

Had thought of adding it into a separate .sql but figured it would work this way too... just adding it as a new line.

[jeremyyeo]

Btw I have some errors when running make unit-test:

Recording: https://asciinema.org/a/VTmYWzD0TvAUO3aaJQh0R5Ell

[beckjake]

@jeremyyeo you may find that rm -rf .tox helps! The next time you run it'll rebuild the environment. It looks like your local tests are potentially running against an old version of dbt.

This PR looks great to me, I'll kick off the rest of the tests and we'll get this merged, assuming everything passes.