-
Notifications
You must be signed in to change notification settings - Fork 84
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
Introduce a context manager that closes opened connections #279
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Thank you for your attention to detail as always.
doc/src/advanced.rst
Outdated
However if an error is raised when selecting the folder, the connection may be | ||
left open. | ||
|
||
IMAPClient provides a context manager that automatically closes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"IMAPClient may be used as a context manager... " might be slightly better?
imapclient/imapclient.py
Outdated
def __exit__(self, exc_type, exc_val, exc_tb): | ||
"""Logout and closes the connection when exiting the context manager. | ||
|
||
All exceptions are caught because an error in `logout` or `shutdown` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"All exceptions during logout and connection shutdown are caught because an error here usually means the connection was already closed."
mock_logger.info.assert_called_once_with( | ||
'Could not close the connection cleanly: %s', | ||
self.client._imap.shutdown.side_effect | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider one more test where an exception occurs inside the with
block. We want to be sure it isn't suppressed by the context manager.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
doc/src/advanced.rst
Outdated
@@ -3,6 +3,33 @@ Advanced Usage | |||
This document covers some more advanced features and tips for handling | |||
specific usages. | |||
|
|||
Prevent leaving unclosed connections |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Cleaning Up Connections" might be better
To prevent users of the library from inadvertently leaking sockets. Fixes mjs#276
612d05c
to
043e4bd
Compare
Awesome stuff. Thank you! |
To prevent users of the library from inadvertently leaking
sockets.
Fixes #276