Replace gevent with requests-futures to address #30 and #40

docs/settings.rst

@@ -16,15 +16,6 @@ customize behavior and improve security.
    process. When enabled, an additional checkbox will be displayed on the
    login form.
 
-.. attribute:: MAMA_CAS_ASYNC_CONCURRENCY
-
-   :default: ``2``
-
-   If single logout is enabled and `gevent`_ is installed, this setting
-   limits the concurrency of requests sent for a logout event. If the number
-   of requests reaches this limit, additional requests block until there is
-   room. Setting this value to zero disables this limiting.
-
 .. attribute:: MAMA_CAS_ATTRIBUTE_CALLBACKS
 
    :default: ``()``
@@ -68,7 +59,7 @@ customize behavior and improve security.
    .. note::
 
       By default, the single logout requests are sent synchronously. If
-      `gevent`_ is installed, they are sent asynchronously.
+      `requests-futures`_ is installed, they are sent asynchronously.
 
    .. warning::
 
@@ -201,4 +192,4 @@ customize behavior and improve security.
    A path to the warning template to use. Make sure Django can find this
    template using normal Django template discovery rules.
 
-.. _gevent: http://www.gevent.org/
+.. _requests-futures: https://github.com/ross/requests-futures

mama_cas/compat.py

@@ -9,12 +9,12 @@
     import xml.etree.ElementTree as etree
 
 
-# gevent is optional, and allows for asynchronous single logout
+# requests-futures is optional, and allows for asynchronous single logout
 # requests. If it is not present, synchronous requests will be sent.
 try:
-    import gevent
+    from requests_futures.sessions import FuturesSession as Session
 except ImportError:  # pragma: no cover
-    gevent = None
+    from requests import Session
 
 
 # defusedxml is optional, and is used for the /samlValidate

mama_cas/models.py

@@ -16,7 +16,7 @@
 
 import requests
 
-from mama_cas.compat import gevent
+from mama_cas.compat import Session
 from mama_cas.exceptions import InvalidProxyCallback
 from mama_cas.exceptions import InvalidRequest
 from mama_cas.exceptions import InvalidService
@@ -34,11 +34,6 @@
 from mama_cas.utils import is_scheme_https
 from mama_cas.utils import match_service
 
-if gevent:
-    from gevent.pool import Pool
-    from gevent import monkey
-    monkey.patch_all(thread=False, select=False)
-
 
 logger = logging.getLogger(__name__)
 
@@ -214,26 +209,12 @@ def request_sign_out(self, user):
         specified user. This is called at logout when single logout
         is enabled.
 
-        If gevent is installed, asynchronous requests will be sent.
-        Otherwise, synchronous requests will be sent. Setting
-        ``MAMA_CAS_ASYNC_CONCURRENCY`` limits concurrent requests for
-        a logout event to the specified value.
+        If requests-futures is installed, asynchronous requests will
+        be sent. Otherwise, synchronous requests will be sent.
         """
-        def spawn(ticket, pool=None):
-            if pool is not None:
-                return pool.spawn(ticket.request_sign_out)
-            return gevent.spawn(ticket.request_sign_out)
-
-        tickets = list(self.filter(user=user, consumed__gte=user.last_login))
-
-        if gevent:
-            size = getattr(settings, 'MAMA_CAS_ASYNC_CONCURRENCY', 2)
-            pool = Pool(size) if size else None
-            sign_out_requests = [spawn(t, pool=pool) for t in tickets]
-            gevent.joinall(sign_out_requests)
-        else:
-            for ticket in tickets:
-                ticket.request_sign_out()
+        session = Session()
+        for ticket in self.filter(user=user, consumed__gte=user.last_login):
+            ticket.request_sign_out(session=session)
 
 
 class ServiceTicket(Ticket):
@@ -263,22 +244,16 @@ def is_primary(self):
             return True
         return False
 
-    def request_sign_out(self):
+    def request_sign_out(self, session=requests):
         """
         Send a POST request to the ``ServiceTicket``s logout URL to
         request sign-out.
         """
-        if not logout_allowed(self.service):
-            return
-        request = SingleSignOutRequest(context={'ticket': self})
-        url = get_logout_url(self.service) or self.service
-        try:
-            resp = requests.post(url, data={'logoutRequest': request.render_content()})
-            resp.raise_for_status()
-        except requests.exceptions.RequestException as e:
-            logger.warning("Single sign-out request to %s returned %s" % (url, e))
-        else:
-            logger.debug("Single sign-out request sent to %s" % url)
+        if logout_allowed(self.service):
+            request = SingleSignOutRequest(context={'ticket': self})
+            url = get_logout_url(self.service) or self.service
+            session.post(url, data={'logoutRequest': request.render_content()})
+            logger.info("Single sign-out request sent to %s" % url)
 
 
 class ProxyTicket(Ticket):

mama_cas/tests/test_models.py

@@ -274,21 +274,7 @@ def test_request_sign_out(self):
         """
         ServiceTicketFactory(consume=True)
         ServiceTicketFactory(consume=True)
-        with patch('requests.post') as mock:
-            mock.return_value.status_code = 200
-            ServiceTicket.objects.request_sign_out(self.user)
-            self.assertEqual(mock.call_count, 2)
-
-    @override_settings(MAMA_CAS_ASYNC_CONCURRENCY=0)
-    def test_request_sign_out_no_pool(self):
-        """
-        Calling the ``request_sign_out()`` manager method with
-        concurrency disabled should issue a POST request for each
-        consumed ticket for the provided user.
-        """
-        ServiceTicketFactory(consume=True)
-        ServiceTicketFactory(consume=True)
-        with patch('requests.post') as mock:
+        with patch('requests.Session.post') as mock:
             mock.return_value.status_code = 200
             ServiceTicket.objects.request_sign_out(self.user)
             self.assertEqual(mock.call_count, 2)
@@ -328,7 +314,7 @@ def test_request_sign_out(self):
         cause any side-effects.
         """
         st = ServiceTicketFactory()
-        with patch('requests.post') as mock:
+        with patch('requests.Session.post') as mock:
             mock.return_value.status_code = 200
             st.request_sign_out()
 
@@ -338,7 +324,7 @@ def test_request_sign_out_exception(self):
         it should be handled.
         """
         st = ServiceTicketFactory()
-        with patch('requests.post') as mock:
+        with patch('requests.Session.post') as mock:
             mock.side_effect = requests.exceptions.RequestException
             st.request_sign_out()
 
@@ -348,7 +334,7 @@ def test_request_sign_out_invalid_status(self):
         status code, the resulting exception should be handled.
         """
         st = ServiceTicketFactory()
-        with patch('requests.post') as mock:
+        with patch('requests.Session.post') as mock:
             mock.return_value.status_code = 500
             st.request_sign_out()
 
@@ -358,7 +344,7 @@ def test_request_sign_out_logout_allow_false(self):
         request should not be sent.
         """
         st = ServiceTicketFactory(service='http://example.com')
-        with patch('requests.post') as mock:
+        with patch('requests.Session.post') as mock:
             mock.return_value.status_code = 500
             st.request_sign_out()
             self.assertEqual(mock.call_count, 0)

mama_cas/tests/test_views.py

@@ -276,7 +276,7 @@ def test_logout_single_sign_out(self):
         ServiceTicketFactory()
         ServiceTicketFactory()
         self.client.login(**self.user_info)
-        with patch('requests.post') as mock:
+        with patch('requests.Session.post') as mock:
             self.client.get(reverse('cas_logout'))
             self.assertEqual(mock.call_count, 2)