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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

PapermillExecutionError cannot be loaded from picklefile #628

Closed
Calder-Ty opened this issue Aug 30, 2021 · 1 comment 路 Fixed by #629
Closed

PapermillExecutionError cannot be loaded from picklefile #628

Calder-Ty opened this issue Aug 30, 2021 · 1 comment 路 Fixed by #629

Comments

@Calder-Ty
Copy link
Contributor

Calder-Ty commented Aug 30, 2021

馃悰 Bug

As part of my team's workflow, we extensively use Papermill to run notebooks as part of our Apache Airflow pipelines. When upgrading Airflow to version 2.X.X we found that our default error handling tools were no longer providing detailed errors from our PapermIll tasks. The cause for this is that Airflow changed the way it handles errors in it's tasks. It temporarily pickles the error message, and then at a later point loads the pickled exception.

Because PapermillExecutionError is called with multiple arguments, but then only passes up a message argument to its superclass, pickle is unable to load it. See this stack overflow question/answer for a general case of this issue.

This error can be recreated with this simple test:

import tempfile
import pickle

from papermill.exceptions import PapermillExecutionError

def test_exceptions_are_unpickleable():
    """Ensure exceptions can be unpickled"""
    temp_file = NamedTemporaryFile()
    err = PapermillExecutionError(1,2, "TestSource", "Exception", Exception(), ["Traceback", "message"])

    with open(temp_file.name, 'wb') as fd:
        pickle.dump(err, fd)
    # Read the Pickled File
    temp_file.seek(0)
    data = temp_file.read()
    pickled_err = pickle.loads(data)
    assert str(pickled_err) == str(err)
@Calder-Ty
Copy link
Contributor Author

The accompanying pull request resolves this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant