IMAP4 initialization api

[filiphanes]

Current connection code looks like:

# creates IMAP4 instace, ImapClientProtocol and task for connection
imap = IMAP4(host, port)
# waits for successful connection (state changes from AUTH to NONAUTH)
await imap.wait_hello_from_server()
# changes state to AUTH
await imap.login(username, password)  # or imap.authenticate

1. new version

# creates IMAP4 instace, does not start connection
imap = IMAP4(loop=myloop, timeout=10)
# creates ImapClientProtocol
# connects to (await loop.create_connection)
await imap.connect(host, port)
# changes state to AUTH
await imap.login(username, password)  # or imap.authenticate

• makes connection initialization later
• connect is more readable and intuitive than wait_hello_from_server
• user can control when connection starts
• user can reconnect after failed connection

2. new version more like imaplib.IMAP4

# creates class, does not start connection
imap = IMAP4(host, port)
# creates ImapClientProtocol
# connection (await loop.create_connection)
# changes state to AUTH
await imap.login(username, password)  # or imap.authenticate

• more like imaplib.IMAP4
• makes connection initialization later
• disadvantage: need to insert code for checking connection to commands LOGIN, AUTHENTICATE, CAPABILITY or even all command

What do you think about these updates?

[bamthomas]

sorry for the delay I was on another projects lately, that makes sense to me, I will see the implication for your third point soon

[aleks-v-k]

Hi! Is there any update on the topic? I'm interested in moving the connection start out of __init__. The current implementation makes it hard to deal with exceptions during the connection process.

[filiphanes]

This issue is about moving connection to 1. connect or 2. __init__.
What Exceptions do you have problem dealing with?

[aleks-v-k]

@filiphanes I'm talking about this:

In [1]: from aioimaplib import aioimaplib

In [2]: host = "someunknown.host.is.here"

In [3]: try:
   ...:     imap_client = aioimaplib.IMAP4_SSL(host=host)
   ...:     await imap_client.wait_hello_from_server()
   ...: except Exception as err:
   ...:     print("Got this:", err)
[E 221212 12:22:49 base_events:1604] Task exception was never retrieved
    future: <Task finished coro=<BaseEventLoop.create_connection() done, defined at /usr/local/lib/python3.7/asyncio/base_events.py:858> exception=gaierror(-2, 'Name or service not known')>
    Traceback (most recent call last):
      File "/usr/local/lib/python3.7/asyncio/base_events.py", line 905, in create_connection
        type=socket.SOCK_STREAM, proto=proto, flags=flags, loop=self)
      File "/usr/local/lib/python3.7/asyncio/base_events.py", line 1275, in _ensure_resolved
        proto=proto, flags=flags)
      File "/usr/local/lib/python3.7/asyncio/base_events.py", line 784, in getaddrinfo
        None, getaddr_func, host, port, family, type, proto, flags)
      File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
        result = self.fn(*self.args, **self.kwargs)
      File "/usr/local/lib/python3.7/socket.py", line 748, in getaddrinfo
        for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
    socket.gaierror: [Errno -2] Name or service not known
Got this: 

In [4]: 

The cause is simple enough. create_client is synchronous (because it's called from __init__). And it creates an asynchronous connection in an asyncio task: https://github.com/bamthomas/aioimaplib/blob/master/aioimaplib/aioimaplib.py#L688
If we get your suggested async connect method (which will create the connection), then it will be straight and simple to handle such exceptions:

async def connect(...):
    # code skipped
    await loop.create_connection(
        lambda: self.protocol, self.host, self.port, ssl=self.ssl_context
    )
    await self.wait_hello_from_server()

in client code, we can easily catch connection exceptions, by just enclosing connect into try-except block.
So, the suggested API changes will solve the problem.

[filiphanes]

yes, I understand you.
I was more inclined to second solution because of "compatibility" with imaplib, but it would require connecting on first command (login, authenticate, ...) which could hide connection errors, or maybe not.

Does anybody else want/need second solution more then first?

[bamthomas]

thank you @aleks-v-k for your comment and @filiphanes for your insights and issue.

At the beginning I think that we've seen the constructor as instantiating objects with no network actions. It has the advantage also to not mix async and non async commands.

So I'd be more inclined to go for the first proposal.