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

Bugfix/pickle errors #629

Merged
merged 3 commits into from Aug 31, 2021
Merged

Bugfix/pickle errors #629

merged 3 commits into from Aug 31, 2021

Conversation

Calder-Ty
Copy link
Contributor

What does this PR do?

Adds the ability to load PapermillExecutionErrors from pickle files.

Airflow 2.0.0 changed the way it handles error reporting by temporarily pickling the exception. Prior to this commit, papermill exceptions could be pickled by airflow, but then not loaded back, causing cryptic error messages.

This pull request addresses this issue, by modifying the call to the superclass, and sending up all the arguments used to create the exception. It also implements a __str__ method, to maintain compatibility with the behavior of calling str() on the exception returning the formatted message.

Fixes #628

This fixes an issue where the PapermillExecutionError can be pickled but will
not be unpicklable.

Pythons BaseException class has a handling to make sure that exceptions are able
to be loaded once they are pickled. This works by ensuring the args used to generate the instance
are saved on the exception. However by only passing the message to the superclass, the pickle
module was unable to load a new instance of PapermillExecutionError from a file.

A new __str__ method was added to preserve the functionality of calling str() on the exception and
only getting the error message back, while ensuring the superclass had access to all instantiating
arguments.
@codecov
Copy link

codecov bot commented Aug 31, 2021

Codecov Report

Merging #629 (df523ba) into main (3f6225e) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #629      +/-   ##
==========================================
+ Coverage   91.63%   91.65%   +0.01%     
==========================================
  Files          17       17              
  Lines        1555     1558       +3     
==========================================
+ Hits         1425     1428       +3     
  Misses        130      130              
Impacted Files Coverage Δ
papermill/exceptions.py 87.87% <100.00%> (+1.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f6225e...df523ba. Read the comment docs.

@MSeal MSeal merged commit a318730 into nteract:main Aug 31, 2021
@MSeal
Copy link
Member

MSeal commented Aug 31, 2021

Thanks for the improvement @Calder-Ty !

@Calder-Ty Calder-Ty deleted the bugfix/pickle_errors branch August 6, 2022 04:06
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.

PapermillExecutionError cannot be loaded from picklefile
2 participants