Remove Python 2 select.select() code (#7964)

In #7882, we started using the much more efficient selectors module with Python 3 instead of select.select(). We may now remove the Python 2 code that was still using the original select.select().

src/python/pants/java/nailgun_executor.py

@@ -5,6 +5,7 @@
 import logging
 import os
 import re
+import selectors
 import threading
 import time
 from contextlib import closing
@@ -20,12 +21,6 @@
 from pants.util.memo import memoized_classproperty
 
 
-if PY3:
-  import selectors
-else:
-  import select
-
-
 logger = logging.getLogger(__name__)
 
 
@@ -220,37 +215,23 @@ def possibly_raise_timeout(remaining_time):
           stderr=stderr,
         )
 
-    if PY3:
-      # NB: We use PollSelector, rather than the more efficient DefaultSelector, because
-      # DefaultSelector results in using the epoll() syscall on Linux, which does not work with
-      # regular text files like ng_stdout. See https://stackoverflow.com/a/8645770.
-      with selectors.PollSelector() as selector, \
-        safe_open(self._ng_stdout, 'r') as ng_stdout:
-        selector.register(ng_stdout, selectors.EVENT_READ)
-        while 1:
-          remaining_time = calculate_remaining_time()
-          possibly_raise_timeout(remaining_time)
-          events = selector.select(timeout=-1 * remaining_time)
-          if events:
-            line = ng_stdout.readline()  # TODO: address deadlock risk here.
-            try:
-              return self._NG_PORT_REGEX.match(line).group(1)
-            except AttributeError:
-              pass
-            accumulated_stdout += line
-    else:
-      with safe_open(self._ng_stdout, 'r') as ng_stdout:
-        while 1:
-          remaining_time = calculate_remaining_time()
-          possibly_raise_timeout(remaining_time)
-          readable, _, _ = select.select([ng_stdout], [], [], (-1 * remaining_time))
-          if readable:
-            line = ng_stdout.readline()  # TODO: address deadlock risk here.
-            try:
-              return self._NG_PORT_REGEX.match(line).group(1)
-            except AttributeError:
-              pass
-            accumulated_stdout += line
+    # NB: We use PollSelector, rather than the more efficient DefaultSelector, because
+    # DefaultSelector results in using the epoll() syscall on Linux, which does not work with
+    # regular text files like ng_stdout. See https://stackoverflow.com/a/8645770.
+    with selectors.PollSelector() as selector, \
+      safe_open(self._ng_stdout, 'r') as ng_stdout:
+      selector.register(ng_stdout, selectors.EVENT_READ)
+      while 1:
+        remaining_time = calculate_remaining_time()
+        possibly_raise_timeout(remaining_time)
+        events = selector.select(timeout=-1 * remaining_time)
+        if events:
+          line = ng_stdout.readline()  # TODO: address deadlock risk here.
+          try:
+            return self._NG_PORT_REGEX.match(line).group(1)
+          except AttributeError:
+            pass
+          accumulated_stdout += line
 
   def _create_ngclient(self, port, stdout, stderr, stdin):
     return NailgunClient(port=port, ins=stdin, out=stdout, err=stderr)

src/python/pants/util/BUILD

@@ -157,9 +157,6 @@ python_binary(
 python_library(
   name = 'socket',
   sources = ['socket.py'],
-  dependencies = [
-    '3rdparty/python:future',
-  ]
 )
 
 python_library(

src/python/pants/util/socket.py

@@ -1,18 +1,10 @@
 # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-import errno
 import io
+import selectors
 import socket
 
-from future.utils import PY3
-
-
-if PY3:
-  import selectors
-else:
-  import select
-
 
 def teardown_socket(s):
   """Shuts down and closes a socket."""
@@ -24,38 +16,16 @@ def teardown_socket(s):
     s.close()
 
 
-# TODO(6071): Remove this once we drop Python 2, because 1) we no longer want to use select.select()
-# in favor of https://docs.python.org/3/library/selectors.html, which uses more efficient and robust
-# algorithms at a better level of abstraction, and because 2) PEP 474 fixed the issue with SIGINT
-# https://www.python.org/dev/peps/pep-0475/.
-def safe_select(*args, **kwargs):
-  # N.B. This while loop is purely to facilitate SA_RESTART-like behavior for select(), which is
-  # (apparently) not covered by signal.siginterrupt(signal.SIGINT, False) when a timeout is passed.
-  # This helps avoid an unhandled select.error(4, 'Interrupted system call') on SIGINT.
-  # See https://bugs.python.org/issue12224 for more info.
-  while 1:
-    try:
-      return select.select(*args, **kwargs)
-    except (OSError, select.error) as e:
-      if e[0] != errno.EINTR:
-        raise
-
-
-# TODO(6071): require kwarg-only args after `fileobj`.
-def is_readable(fileobj, timeout=None):
+def is_readable(fileobj, *, timeout=None):
   """Check that the file-like resource is readable within the given timeout via polling.
   :param Union[int, SupportsFileNo] fileobj:
   :param Optional[int] timeout: (in seconds)
   :return bool
   """
-  if PY3:
-    with selectors.DefaultSelector() as selector:
-      selector.register(fileobj, selectors.EVENT_READ)
-      events = selector.select(timeout=timeout)
-    return bool(events)
-  else:
-    readable, _, _ = safe_select([fileobj], [], [], timeout)
-    return bool(readable)
+  with selectors.DefaultSelector() as selector:
+    selector.register(fileobj, selectors.EVENT_READ)
+    events = selector.select(timeout=timeout)
+  return bool(events)
 
 
 class RecvBufferedSocket:

tests/python/pants_test/java/test_nailgun_executor.py

@@ -81,7 +81,7 @@ def test_connect_timeout(self):
          unittest.mock.patch('pants.java.nailgun_executor.read_file') as mock_read_file:
       mock_open.return_value = stdout_read
       mock_read_file.return_value = 'err'
-      # The stdout write pipe has no input and hasn't been closed, so the select.select() should
+      # The stdout write pipe has no input and hasn't been closed, so the selector.select() should
       # time out regardless of the timemout argument, and raise.
       with self.assertRaisesWithMessage(NailgunExecutor.InitialNailgunConnectTimedOut, """\
 Failed to read nailgun output after 0.0001 seconds!

tests/python/pants_test/util/BUILD

@@ -144,7 +144,6 @@ python_tests(
   sources = ['test_socket.py'],
   coverage = ['pants.util.socket'],
   dependencies = [
-    '3rdparty/python:future',
     'src/python/pants/util:socket',
   ]
 )

tests/python/pants_test/util/test_socket.py

@@ -6,8 +6,6 @@
 import unittest
 import unittest.mock
 
-from future.utils import PY3
-
 from pants.util.socket import RecvBufferedSocket, is_readable
 
 
@@ -16,23 +14,17 @@
 
 class TestSocketUtils(unittest.TestCase):
 
-  @unittest.mock.patch('selectors.DefaultSelector' if PY3 else 'select.select', **PATCH_OPTS)
+  @unittest.mock.patch('selectors.DefaultSelector', **PATCH_OPTS)
   def test_is_readable(self, mock_selector):
     mock_fileobj = unittest.mock.Mock()
-    if PY3:
-      mock_selector = mock_selector.return_value.__enter__.return_value
-      mock_selector.register = unittest.mock.Mock()
-      # NB: the return value should actually be List[Tuple[SelectorKey, Events]], but our code only
-      # cares that _some_ event happened so we choose a simpler mock here. See
-      # https://docs.python.org/3/library/selectors.html#selectors.BaseSelector.select.
-      mock_selector.select = unittest.mock.Mock(return_value=[(1, "")])
-    else:
-      mock_selector.return_value = ([1], [], [])
+    mock_selector = mock_selector.return_value.__enter__.return_value
+    mock_selector.register = unittest.mock.Mock()
+    # NB: the return value should actually be List[Tuple[SelectorKey, Events]], but our code only
+    # cares that _some_ event happened so we choose a simpler mock here. See
+    # https://docs.python.org/3/library/selectors.html#selectors.BaseSelector.select.
+    mock_selector.select = unittest.mock.Mock(return_value=[(1, "")])
     self.assertTrue(is_readable(mock_fileobj, timeout=0.1))
-    if PY3:
-      mock_selector.select = unittest.mock.Mock(return_value=[])
-    else:
-      mock_selector.return_value = ([], [], [])
+    mock_selector.select = unittest.mock.Mock(return_value=[])
     self.assertFalse(is_readable(mock_fileobj, timeout=0.1))
 
 
@@ -64,17 +56,14 @@ def test_recv_max_larger_than_buf(self):
     self.server_sock.sendall(b'A' * double_chunk)
     self.assertEqual(self.buf_sock.recv(double_chunk), b'A' * double_chunk)
 
-  @unittest.mock.patch('selectors.DefaultSelector' if PY3 else 'select.select', **PATCH_OPTS)
+  @unittest.mock.patch('selectors.DefaultSelector', **PATCH_OPTS)
   def test_recv_check_calls(self, mock_selector):
-    if PY3:
-      mock_selector = mock_selector.return_value.__enter__.return_value
-      mock_selector.register = unittest.mock.Mock()
-      # NB: the return value should actually be List[Tuple[SelectorKey, Events]], but our code only
-      # cares that _some_ event happened so we choose a simpler mock here. See
-      # https://docs.python.org/3/library/selectors.html#selectors.BaseSelector.select.
-      mock_selector.select = unittest.mock.Mock(return_value=[(1, "")])
-    else:
-      mock_selector.return_value = ([1], [], [])
+    mock_selector = mock_selector.return_value.__enter__.return_value
+    mock_selector.register = unittest.mock.Mock()
+    # NB: the return value should actually be List[Tuple[SelectorKey, Events]], but our code only
+    # cares that _some_ event happened so we choose a simpler mock here. See
+    # https://docs.python.org/3/library/selectors.html#selectors.BaseSelector.select.
+    mock_selector.select = unittest.mock.Mock(return_value=[(1, "")])
 
     self.mock_socket.recv.side_effect = [b'A' * self.chunk_size, b'B' * self.chunk_size]