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

fix: add reduce to allow errors to be pickled #148

Merged
merged 2 commits into from
Feb 10, 2021

Conversation

bcvandendool
Copy link
Contributor

Fixes

Fixes #139
When an error occurs when running the library in a celery shared task, celery tries to pickle the error which fails. This is because the init function requires a parameter that it cannot automatically reconstruct from the info stored in the class. This modifies the init function to allow both the original way of initializing, as well as being able to specify all the arguments separately. The reduce function has been implemented to explicitly tell it how it should pickle this class, and which parameters to use. A test has been added to show that the errors can now be pickled.

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Jan 12, 2021
@codecov
Copy link

codecov bot commented Jan 12, 2021

Codecov Report

Merging #148 (c75074c) into main (6d62911) will increase coverage by 0.32%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #148      +/-   ##
==========================================
+ Coverage   95.14%   95.46%   +0.32%     
==========================================
  Files           6        6              
  Lines         309      331      +22     
==========================================
+ Hits          294      316      +22     
  Misses         15       15              
Impacted Files Coverage Δ
python_http_client/exceptions.py 97.82% <100.00%> (+0.39%) ⬆️
tests/test_unit.py 99.33% <100.00%> (+0.07%) ⬆️

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 6d62911...c75074c. Read the comment docs.

@bcvandendool bcvandendool force-pushed the fix-error-pickling branch 3 times, most recently from 57eed5e to f44abad Compare January 12, 2021 20:09
python 2 adds a __cause__ that is None to the __dict__ of the thrown error, failing the assertion
@JenniferMah JenniferMah changed the title <fix>: add reduce to allow errors to be pickled fix: add reduce to allow errors to be pickled Jan 13, 2021
Copy link
Contributor

@JenniferMah JenniferMah left a comment

Choose a reason for hiding this comment

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

Thanks for your PR @bcvandendool!

tests/test_unit.py Show resolved Hide resolved
@thinkingserious thinkingserious merged commit dafd024 into sendgrid:main Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pickle / celery breaks with exceptions
3 participants