Repository: mjs/imapclient

Issue:
Accessing the socket used by `IMAPClient`
To watch a mailbox using IDLE, the current documentation suggests [using a busyloop](https://imapclient.readthedocs.io/en/2.2.0/advanced.html#watching-a-mailbox-using-idle) with a relatively small timeout to check for responses. A small timeout because `IMAPClient.idle_check` uses a [blocking poll](https://github.com/mjs/imapclient/blob/master/imapclient/imapclient.py#L936) with this timeout and handling `KeyboardInterrup` is not possible while it blocks. An alternative to the busy loop was suggested in <https://github.com/mjs/imapclient/issues/322>, using `select.select` to poll both the connecting socket and a separate socket.

However, in order to poll the socket, access to the underlying socket is required, available as `IMAPClient._sock`. My understanding is that this attribute is an implementation detail. Can this be relied upon? If not, is it possible to make the socket available as part of the API?

Discussion:
mjs: I'm find with exposing the socket. Thank you for including suitable warnings in the docstring.

I will try and test with Python 3.4. My laptop doesn't currently have old Python versions installed (or Docker) but I need to get that sorted out anyway.

The example makes sense but I wonder if it could be simpler. Does it really need to use threading?

I agree that keeping the existing simpler IDLE example is probably best. A link in the docs to the select based example might be nice. 

NicolasLM: I agree that the example seems complicated. Isn't it possible to do the same thing on the main thread?

Also `select` is a low level API with sharp edges. As an example of an issue I ran into, most kernels are compiled with select being able to work with file descriptors with an ID lesser than 1025. So if your application opens a bunch of files, then opens a socket, this socket can get ID 1060 and won't work with select. The call to `select.select([socket], [], [])` will return an error.

That is why I would rather see this example use the `selectors` module which picks the best primitive according to the OS instead of relying on the common denominator.
BoniLindsley: I have changed the example to use `selectors`. I will clean-up the history with another force-push once everything is okay.

However, I now feel it is a bad example of `socket()` usage. Other than a slightly earlier detection of disconnect, it is mostly equivalent to directly using `while response := idle_check(remaining_time)` which already polls. I think the use of `socket()` only really shines in an event handling loop. Three potential options come to mind:

- Use the `selector` example as is. Readers might wonder "why poll at all?"
- Make an `asyncio` example. Likely lengthy. Not easily compatible with Windows which has no asyncio signal handling.
- Drop the example. It is a know-what-you-are-doing API. Users might be less clear on what is allowed with `socket()`.
NicolasLM: I agree that IDLE in synchronous Python feels weird and is much better used as part of an event loop. That being said users are going to be super confused if we steer them in the direction of using IMAPClient synchronously for establishing connections and then switching to a framework like asyncio for IDLE. I did just that at a previous job to monitor thousands of mailboxes and people stumbling upon the code were always very confused.

With unlimited time we could remove the use of imaplib, split the code into IO/sans-IO and let users decide if they want synchronous or async code.

So let's merge that for now and see later if we can provide a better user interface for IDLE.
NicolasLM: @BoniLindsley Could you squash your commits please?

Patch:
diff --git a/examples/idle_selector_example.py b/examples/idle_selector_example.py
new file mode 100644
index 0000000..e8ff729
--- /dev/null
+++ b/examples/idle_selector_example.py
@@ -0,0 +1,36 @@
+from datetime import datetime, timedelta
+from selectors import DefaultSelector, EVENT_READ
+
+from imapclient import IMAPClient
+
+HOST = "localhost"
+USERNAME = "user"
+PASSWORD = "Tr0ub4dor&3"
+RESPONSE_TIMEOUT_SECONDS = 15
+IDLE_SECONDS = 60 * 24
+
+with IMAPClient(HOST, timeout=RESPONSE_TIMEOUT_SECONDS) as server:
+    server.login(USERNAME, PASSWORD)
+    server.select_folder("INBOX", readonly=True)
+    server.idle()
+    print(
+        "Connection is now in IDLE mode,"
+        " send yourself an email or quit with ^c"
+    )
+    try:
+        with DefaultSelector() as selector:
+            selector.register(server.socket(), EVENT_READ, None)
+            now = datetime.now
+            end_at = now() + timedelta(seconds=IDLE_SECONDS)
+            while selector.select((end_at - now()).total_seconds()):
+                responses = server.idle_check(timeout=0)
+                if not responses:
+                    raise ConnectionError(
+                        "Socket readable without data. Likely closed."
+                    )
+                print("Server sent:", responses)
+        print("IDLE time out.")
+    except KeyboardInterrupt:
+        print("")  # Newline after the typically echoed ^C.
+    server.idle_done()
+    print("IDLE mode done")
diff --git a/imapclient/imapclient.py b/imapclient/imapclient.py
index 8d58693..a7bc703 100644
--- a/imapclient/imapclient.py
+++ b/imapclient/imapclient.py
@@ -11,6 +11,7 @@ import select
 import socket
 import sys
 import re
+import warnings
 from collections import namedtuple
 from datetime import datetime, date
 from operator import itemgetter
@@ -329,10 +330,26 @@ class IMAPClient(object):
 
     def _set_read_timeout(self):
         if self._timeout is not None:
-            self._sock.settimeout(self._timeout.read)
+            self.socket().settimeout(self._timeout.read)
 
     @property
     def _sock(self):
+        warnings.warn("_sock is deprecated. Use socket().", DeprecationWarning)
+        return self.socket()
+
+    def socket(self):
+        """Returns socket used to connect to server.
+
+        The socket is provided for polling purposes only.
+        It can be used in,
+        for example, :py:meth:`selectors.BaseSelector.register`
+        and :py:meth:`asyncio.loop.add_reader` to wait for data.
+
+        .. WARNING::
+           All other uses of the returned socket are unsupported.
+           This includes reading from and writing to the socket,
+           as they are likely to break internal bookkeeping of messages.
+        """
         # In py2, imaplib has sslobj (for SSL connections), and sock for non-SSL.
         # In the py3 version it's just sock.
         return getattr(self._imap, "sslobj", self._imap.sock)
@@ -919,7 +936,7 @@ class IMAPClient(object):
              (1, b'EXISTS'),
              (1, b'FETCH', (b'FLAGS', (b'\\NotJunk',)))]
         """
-        sock = self._sock
+        sock = self.socket()
 
         # make the socket non-blocking so the timeout can be
         # implemented for this call


Test patch:
diff --git a/tests/test_imapclient.py b/tests/test_imapclient.py
index 4a065de..d42db68 100644
--- a/tests/test_imapclient.py
+++ b/tests/test_imapclient.py
@@ -7,6 +7,7 @@ from __future__ import unicode_literals
 import itertools
 import socket
 import sys
+import warnings
 from datetime import datetime
 import logging
 
@@ -1055,3 +1056,12 @@ class TestProtocolError(IMAPClientTest):
 
         with self.assertRaises(ProtocolError):
             client._consume_until_tagged_response(sentinel.tag, b"IDLE")
+
+class TestSocket(IMAPClientTest):
+    def test_issues_warning_for_deprecating_sock_property(self):
+        mock_sock = Mock()
+        self.client._imap.sock = self.client._imap.sslobj = mock_sock
+        with warnings.catch_warnings(record=True) as warnings_caught:
+            warnings.simplefilter("always", DeprecationWarning)
+            assert self.client._sock == self.client.socket()
+            assert len(warnings_caught) == 1