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 exceptions picklability #1044

Closed
wants to merge 2 commits into from
Closed

Conversation

amw
Copy link

@amw amw commented Oct 3, 2016

Currently exceptions created by botocore fail when sent through pickle.loads(pickle.dumps(err)). pickle is a module commonly used for serialization of python objects sent between runs or processes. One notable usage example is in multiprocessing module which in turn is used in Django's parallel test suite.

Any user who currently performs boto3 operations inside Django's test runner risks that his test suite blows up without running appropriate teardown operations. I've filed a separate fix for Django in django/django#7325.

These commits fix picklability of botocore exception classes. The culprit seemed to be calling parent Exception.__init__ for defining exception's string representation instead of implementing our own more specialized __str__ method.

One unrelated thing that surprised me while going through the code is that ClientError does not inherit BotoCoreError. I was 100% expecting to be able to catch all AWS-related errors with a single error class, but it seems that I would have to write something like this instead:

try:
    s3_object.load()
except (BotoCoreError, ClientError) as e:
    ...

Is the reason for this to shift blame between AWS/Client? If so I think there would be a room here for another error class that would be parent for both of these.

@amw
Copy link
Author

amw commented Oct 3, 2016

Two issues with my PR:

  1. Turns out my fix does not work with Python 2.x. I'm not sure how to work with Python2's pickling.
  2. I've accidentally omitted ConnectionClosedError which has yet another inheritance tree (from "requests" package) and also fails pickling.

@JordonPhillips
Copy link
Contributor

Thanks for the pr! Unfortunately we will not be able to accept this until it works on all of our supported python versions.

@amw
Copy link
Author

amw commented Oct 27, 2016

I've started to think that aiming for picklability of all errors is pointless especially when you don't catch and translate errors from other libraries that you depend on. Libraries like Django test suite that use multiprocessing should just handle errors more gracefully. I'm closing this PR.

@amw amw closed this Oct 27, 2016
@amw amw deleted the fix-exceptions-picklability branch January 3, 2017 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants