Allow more imap configuration via autofiler config #26
Conversation
doc/source/configuring.rst
Outdated
| the server. | ||
| he server. | ||
|
|
||
| You can also optionally provide the imap servers port and a custom CA file. |
There was a problem hiding this comment.
Since you'll need to update this PR to make flake8 happy anyway, could you change "imap" to "IMAP" here, please?
There was a problem hiding this comment.
Yup, happy to change, also looks like I chopped off the t in the, I'll fix that as well. Other than flake8 are there any other linters you would like me to run this through?
There was a problem hiding this comment.
The tests run by travis are sufficient, thanks!
imapautofiler/client.py
Outdated
| @@ -113,15 +114,23 @@ def expunge(self): | |||
| def close(self): | |||
| "Close the connection, flushing any pending changes." | |||
|
|
|||
|
|
|||
| '/Users/olsenmo/Desktop/internal.pem' | |||
There was a problem hiding this comment.
Was this literal string for debugging?
There was a problem hiding this comment.
Looks like something got pasted in the wrong window. Sorry about that, I'll remove.
| self._conn = imapclient.IMAPClient( | ||
| cfg['server']['hostname'], | ||
| use_uid=True, | ||
| ssl=True, | ||
| port=cfg['server'].get('port'), |
There was a problem hiding this comment.
Is passing None explicitly the same as the default for port? Or should we set the default explicitly in the get() call so we always pass a valid number?
There was a problem hiding this comment.
The reason I used None instead of the default was that it allows the library we are calling the ability to change the default value (say if it became a different type for example). Also the default for this value varies depending on whether ssl is True or not. If we hand the "default" value then any changes that would make ssl optional in the future would need to be aware of this.
There was a problem hiding this comment.
OK. I'm asking about what the underlying library actually expects or supports. I don't have the library code handy to look at what it's doing with a value of None.
There was a problem hiding this comment.
Ah, that makes sense. Here is what the init definition looks like:
def __init__(self, host, port=None, use_uid=True, ssl=True, stream=False,
ssl_context=None, timeout=None):
If port is None it will be set based on what the value of ssl is set to.
There was a problem hiding this comment.
OK, it sounds like None is fine as a default then. Thanks for verifying!
This patch allows you to specify the port used for your imap connection as well as specify a custom CA file. This is useful if the company that setup your imap server is using non standard ports and/or has self signed certs.
digwanderlust
force-pushed
the
master
branch
from
a6a33c7
to
abcf856
Compare
|
I cleaned up the .eggs directory in a separate patch. Thanks for working on this! |
|
Thanks for merging the change so quickly. |
This patch allows you to specify the port used for
your imap connection as well as specify a custom
CA file. This is useful if the company that setup
your imap server is using non standard ports
and/or has self signed certs.