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

Improving logging of Jupyter task to capture cell output #4283

Merged
merged 1 commit into from Mar 24, 2021

Conversation

kvnkho
Copy link
Contributor

@kvnkho kvnkho commented Mar 22, 2021

Summary

This is a response to #4265, which was raised on Slack. The user wanted to capture Jupyter notebook cell outputs in the Prefect logs.

Changes

There are two things needed to get this to work. One is to pass the log_output bool into papermill's execute_notebook function. By default this is False. It needs to be True. For consistency with the previous implementation, I kept the default asFalse, though I feel there might be benefits in making it True.

The second thing is to get the papermill logger and change the level to INFO or DEBUG. By default, it is at WARN so it does not log cell outputs. I think we should not do this change on the task side. Instead, Prefect users should use the (extra loggers)[https://docs.prefect.io/core/concepts/logging.html#extra-loggers] part of the TOML config and add the papermill logger.

There is an added test to see if the papermill logs are being captured. There are some edits to the .ipynb to see more output from the papermill logs.

Checklist

This PR:

  • adds new tests (if appropriate)
  • adds a change file in the changes/ directory (if appropriate)
  • updates docstrings for any new functions or function arguments, including docs/outline.toml for API reference docs (if appropriate)

@kvnkho kvnkho requested a review from jcrist as a code owner March 22, 2021 22:47
@marvin-robot
Copy link
Member

Here I am, brain the size of a planet and they ask me to welcome you to Prefect.

So, welcome to the community @kvnkho! 🎉 🎉

Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

LGTM and congrats on your first PR!

@cicdw cicdw merged commit 300d718 into PrefectHQ:master Mar 24, 2021
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.

None yet

3 participants