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

Added no-print flag #4854

Merged
merged 7 commits into from
Apr 11, 2022
Merged

Added no-print flag #4854

merged 7 commits into from
Apr 11, 2022

Conversation

poloaraujo
Copy link
Contributor

@poloaraujo poloaraujo commented Mar 10, 2022

resolves #4710

Description

Add a flag to suppress {{ print(msg) }} messages coming out of macros.

print functionality in macros was added in as part of #3451 but there is currently no way to keep the messages from being printed to stdout. This can result in these messages interspersed with logs going to stdout; macros using print() will always have the messages they are printing as an output.

16:31:35  Running with dbt=1.0.1
16:31:35  Found 5 models, 20 tests, 0 snapshots, 0 analyses, 180 macros, 1 operation, 3 seed files, 0 sources, 0 exposures, 0 metrics
16:31:35
16:31:40
16:31:40  Running 1 on-run-start hook
16:31:40  ********************This is a LOG line!
********************This is a PRINT line!

Describe alternative you've considered

Not doing this simply means that {{ print(msg) }} will always print to stdout.

Who will this benefit?

Right now, anything sent to stdout using {{ print(msg) }} will get printed to stdout all the time with no way to suppress it. This will allow anyone using macros utilizing print to suppress those messages.

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 added information about my change to be included in the CHANGELOG.

@cla-bot cla-bot bot added the cla:yes label Mar 10, 2022
@poloaraujo poloaraujo marked this pull request as ready for review March 24, 2022 13:39
@poloaraujo poloaraujo requested review from a team as code owners March 24, 2022 13:39
CHANGELOG.md Outdated
@@ -18,6 +18,7 @@
- Allow unique_key to take a list ([#2479](https://github.com/dbt-labs/dbt-core/issues/2479), [#4618](https://github.com/dbt-labs/dbt-core/pull/4618))
- Add `--quiet` global flag and `print` Jinja function ([#3451](https://github.com/dbt-labs/dbt-core/issues/3451), [#4701](https://github.com/dbt-labs/dbt-core/pull/4701))
- Add space before justification periods ([#4737](https://github.com/dbt-labs/dbt-core/issues/4737), [#4744](https://github.com/dbt-labs/dbt-core/pull/4744))
- Add `--no-print` global flag ([#4710](https://github.com/dbt-labs/dbt-core/issues/4710))
Copy link
Member

Choose a reason for hiding this comment

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

We've updated the process we use to generate changelogs so that you no longer have to manually figure out where they go. That's also what the cause of the failing GitHub action is. You'll need to delete this line and then go through the new process. You can see the details of how to do it in our Contributing guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @emmyoop! I've updated the changelog following the new process and I've just pushed the changes.

Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

Sorry this has been sitting for so long! It's looking great. Once you fix the changelog issue and the tests run successfully I'll take another look.

@nathaniel-may nathaniel-may self-requested a review March 31, 2022 19:06
@sansar-choinyambuu
Copy link

@poloaraujo do you think you can do changes mentioned by @emmyoop regarding the CHANGELOG.md ? We need this PR merged ;)

@poloaraujo
Copy link
Contributor Author

Hi @sansar-choinyambuu and @emmyoop! I'm sorry for the huge delay. I've updated the changelog following the new process and I've pushed it!

@poloaraujo poloaraujo requested a review from emmyoop April 8, 2022 10:47
Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

Looks good! I suggested just a small change to the changelog entry. The yaml file can be modified. No need to re-run the changie command!

.changes/unreleased/Features-20220408-114118.yaml Outdated Show resolved Hide resolved
@poloaraujo poloaraujo requested a review from emmyoop April 11, 2022 17:13
@poloaraujo
Copy link
Contributor Author

@emmyoop thanks! I've committed your suggestion.

Copy link
Contributor

@nathaniel-may nathaniel-may left a comment

Choose a reason for hiding this comment

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

LGTM

@emmyoop emmyoop merged commit 827eae2 into dbt-labs:main Apr 11, 2022
VersusFacit pushed a commit that referenced this pull request Apr 14, 2022
* Added no-print flag

* Updated changelog

* Updated changelog

* Removed changes from CHANGELOG.md

* Updated CHANGELOG.MD with changie

* Update .changes/unreleased/Features-20220408-114118.yaml

Co-authored-by: Emily Rockman <[email protected]>

Co-authored-by: Emily Rockman <[email protected]>
agoblet pushed a commit to BigDataRepublic/dbt-core that referenced this pull request May 20, 2022
* Added no-print flag

* Updated changelog

* Updated changelog

* Removed changes from CHANGELOG.md

* Updated CHANGELOG.MD with changie

* Update .changes/unreleased/Features-20220408-114118.yaml

Co-authored-by: Emily Rockman <[email protected]>

Co-authored-by: Emily Rockman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-217] add --no-print flag
5 participants