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

Add log_prints option to redirect print to logs #7580

Merged
merged 11 commits into from
Nov 30, 2022
Merged

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Nov 17, 2022

Addresses the much requested log_stdout feature from v1 in v2 by adding a log_prints option. It is very hard for us to log standard output from within the run as we have no way to tell where the bytes are coming from (i.e. you cannot disable capturing per task or flow). We can, however, patch the builtin print method, allowing us to identify the flow or task run the message comes from.

This pull request introduces temporary patching of print to accomplish logging of prints from within tasks or flows. This will also apply to any functions within the scope of a task or a flow e.g. if they call into another library. If a flow or task run context is not present, we will just print as normal.

There are options at the flow, task, and settings level. Defaults for children (subflows and tasks) are pulled from the parent flow.

Closes #7117

Example

Play with the following, try None, False and True for values:

import prefect


@prefect.flow(log_prints=False)
def foo():
    print("foo")
    bar()


@prefect.task(log_prints=True)
def bar():
    print("bar")


foo()

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • This pull request includes a label categorizing the change e.g. fix, feature, enhancement

Future work

We may add a log_stdout feature but it would operate at the infrastructure level where we actually have access to the full standard output stream.

@netlify
Copy link

netlify bot commented Nov 17, 2022

Deploy Preview for prefect-orion ready!

Name Link
🔨 Latest commit aed7ba9
🔍 Latest deploy log https://app.netlify.com/sites/prefect-orion/deploys/63879533540aa800084fb25a
😎 Deploy Preview https://deploy-preview-7580--prefect-orion.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@zanieb zanieb added the enhancement An improvement of an existing feature label Nov 17, 2022
@zanieb
Copy link
Contributor Author

zanieb commented Nov 17, 2022

Needs:

  • Tests
  • Link the the closed issue
  • Prose documentation

@zanieb
Copy link
Contributor Author

zanieb commented Nov 18, 2022

Nate is leading tests for this.
Will is leading documentation.

@anna-geller
Copy link
Contributor

anna-geller commented Nov 27, 2022

works 🎉

from prefect import flow

@flow(log_prints=True)
def hello():
    print("Greetings from Prefect! 🤗")

if __name__ == "__main__":
    hello()

image

from prefect import flow, task

@task(log_prints=True)
def print_sth_nice():
    print("This is nice")


@flow(log_prints=True)
def hello():
    print("Greetings from Prefect! 🤗")
    print_sth_nice()

if __name__ == "__main__":
    hello()

image

from prefect import flow, task

@task
def print_sth_nice():
    print("This is nice")


@flow
def hello():
    print("Greetings from Prefect! 🤗")
    print_sth_nice()

if __name__ == "__main__":
    hello()

image

from prefect import flow, task

@task(log_prints=False)
def print_sth_nice():
    print("This is nice")


@flow(log_prints=True)
def hello():
    print("Greetings from Prefect! 🤗")
    print_sth_nice()

if __name__ == "__main__":
    hello()

image

@zanieb zanieb marked this pull request as ready for review November 28, 2022 19:06
@zanieb zanieb requested a review from tpdorsey as a code owner November 28, 2022 19:06
Copy link
Contributor

@anna-geller anna-geller left a comment

Choose a reason for hiding this comment

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

QA passes

@@ -807,6 +816,11 @@ def task(
result_serializer: An optional serializer to use to serialize the result of this
task for persistence. Defaults to the value set in the flow the task is
called in.
timeout_seconds: An optional number of seconds indicating a maximum runtime for
Copy link
Contributor

Choose a reason for hiding this comment

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

😅

Copy link
Contributor

@peytonrunyan peytonrunyan left a comment

Choose a reason for hiding this comment

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

LGTM! The only thing is possibly a test explicitly for checking that the default behavior is False?

Also, TIL about the builtins module. I'd always thought "builtins" was just a colloquialism.

src/prefect/tasks.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tpdorsey tpdorsey left a comment

Choose a reason for hiding this comment

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

LGTM. Only reviewed docs and docstrings.

src/prefect/flows.py Outdated Show resolved Hide resolved
Copy link
Contributor

@bunchesofdonald bunchesofdonald left a comment

Choose a reason for hiding this comment

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

I like it!

Co-authored-by: Terrence Dorsey <[email protected]>
@zanieb
Copy link
Contributor Author

zanieb commented Nov 30, 2022

Thanks for the help everyone!

@zanieb zanieb merged commit 298554b into main Nov 30, 2022
@zanieb zanieb deleted the feature-log-prints branch November 30, 2022 17:39
github-actions bot pushed a commit that referenced this pull request Dec 1, 2022
Co-authored-by: Will Raphaelson <[email protected]>
Co-authored-by: Will Raphaelson <[email protected]>
Co-authored-by: Nathan Nowack <[email protected]>
Co-authored-by: Terrence Dorsey <[email protected]>
github-actions bot pushed a commit to ddelange/prefect that referenced this pull request Dec 15, 2022
Co-authored-by: Will Raphaelson <[email protected]>
Co-authored-by: Will Raphaelson <[email protected]>
Co-authored-by: Nathan Nowack <[email protected]>
Co-authored-by: Terrence Dorsey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stream print statements as logs similarly to the log_stdout in v1
7 participants