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

[CT-1763] [Feature] Configurable behavior for print #6543

Closed
Tracked by #6356
jtcohen6 opened this issue Jan 8, 2023 · 5 comments
Closed
Tracked by #6356

[CT-1763] [Feature] Configurable behavior for print #6543

jtcohen6 opened this issue Jan 8, 2023 · 5 comments
Labels
enhancement New feature or request Impact: Exp logging python_api Issues related to dbtRunner Python entry point Refinement Maintainer input needed

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Jan 8, 2023

In addition to the simple default behavior (Python's print function → stdout), which is fine for CLI use, we should support programmatic invocations, where printing to stdout doesn't make sense. We could do this with a "print" event/log instead, probably with a special level.

If we do go the logging route, we'd need to respect these flags:

  • --quiet: suppress all log messages from stdout, except those with the special "print" level
  • --no-print: suppress all "print"-level log messages from stdout

Implementation options:

  1. In all current call sites for print, we add conditional logic that either prints, or fires an event specific to that call site. (E.g. CustomMacroPrint, ListResult)
  2. We add a single new event type, PrintMessage. We swap out 1:1 all direct calls to print with a new custom "print" method, defined either in clients.system or the events module, and define the conditional logic (to print or not to print) one time in that method.

Current call sites

There are three primary places in the codebase where we call print today, plus a few miscellaneous others.

1. print dbt-Jinja context member

a.k.a. the "codegen" use case

@contextmember
@staticmethod
def print(msg: str) -> str:
"""Prints a line to stdout.
:param msg: The message to print
> macros/my_log_macro.sql
{% macro some_macro(arg1, arg2) %}
{{ print("Running some_macro: " ~ arg1 ~ ", " ~ arg2) }}
{% endmacro %}"
"""
if not flags.NO_PRINT:
print(msg)
return ""

This should optionally fire an event, instead of just calling print.

2. dbt list

We definitely need a way to make the results of list available programmatically. But that might already be the case—we already both print the results, and return them as the task result:

def output_results(self, results):
for result in results:
self.node_results.append(result)
print(result)
return self.node_results

So when the new CLI/API supports list (#5549), we should expect the list results being returned like:

from dbt.cli.main import dbtRunner
dbt = dbtRunner()
results, success = dbt.invoke(['list', '--select', 'some_model+'])
# 'results' now contains the list of selected nodes

In the spirit of completeness, I think it makes sense to opt for sending a structured logging event for each list result, instead of printing it to stdout (default). It should be possible to suppress those in-lieu-of-printing events via the --no-print flag described above.

3. dbt debug

This is pretty clearly a case where we want actual events/logs, and not just text printed to stdout.

Already open in #5353
Proposed resolution in #6541

4. Misc other spots

In dbt init — this is probably just an oversight, I believe there should be an event manager available:

print("No adapters available. Go to https://docs.getdbt.com/docs/available-adapters")

In our functional testing framework — nbd

@jtcohen6 jtcohen6 added enhancement New feature or request python_api Issues related to dbtRunner Python entry point logging labels Jan 8, 2023
@github-actions github-actions bot changed the title [Feature] Configurable behavior for print [CT-1763] [Feature] Configurable behavior for print Jan 8, 2023
@peterallenwebb
Copy link
Contributor

Does this overlap with #6481, our message-only "FYI" event type?

@jtcohen6
Copy link
Contributor Author

@peterallenwebb Hmm - maybe?

The "printed" output of dbt list is intended to be parse-able programmatically. The goal here is providing alternative behavior that lets us emulate the user experience of printing something to stdout (i.e. seeing it appear in the "terminal" of the IDE), without actually relying on direct access to sys.stdout (as Python stdlib print() does).

As outlined in the issue above, we may want all current instances of Python print() to fire a generic PrintMessage event, which could be neatly folded into the "FYI" type. But I think we may want each of the use cases identified above (dbt list versus custom {{ print() }}) to fire its own event type. I'm also not sure that the "FYI" type will want to respect --quiet/--no-print.

@ChenyuLInx
Copy link
Contributor

ChenyuLInx commented Feb 28, 2023

Talked about this live, the path forward would be

  • replace all print() call sites with a new Print event type
  • add additional logic in EventManager / std logger, to make sure we don't format Print events (add timestamp etc)
  • add conditional logic to still respect --print/--no-print flag for this Print event type only

Will create new implementation ticket for it.

@ChenyuLInx ChenyuLInx added the Refinement Maintainer input needed label Feb 28, 2023
@jtcohen6 jtcohen6 reopened this Sep 28, 2023
@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Oct 2, 2023

Let's open as a new issue.

We believe dbt debug has been resolved.

Targeting the two important remaining pieces:

  1. User print method. We want this to always be a logging event, but without timestamp/formatting in stdout. Convert to UserPrint event.
  2. dbt list with non-JSON logging. We want this to always be a logging event, but without timestamp/formatting in stdout. Convert to always use ListCmdOut.

So we need new behavior for the text-formatted stdout logger, to include timestamp/formatting by default, but not for:

  • UserPrint or ListOutput events

Acceptance criteria:

  • Tests for stdout text-formatted, stdout JSON-formatted, logs/dbt.log file
  • Catalog any remaining instances of Python print method within dbt-core codebase

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Oct 2, 2023

Closing in favor of implementation ticket:

@jtcohen6 jtcohen6 closed this as completed Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Impact: Exp logging python_api Issues related to dbtRunner Python entry point Refinement Maintainer input needed
Projects
None yet
Development

No branches or pull requests

4 participants