geoip: move from requests to aiohttp again

python3-aiohttp has been fixed and released in focal-updates.
New Subiquity builds should not be affected by LP: #1969393 anymore.

This reverts commit 413cc87337.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>

apt-deps.txt

@@ -15,6 +15,7 @@ os-prober
 pep8
 pkg-config
 python3-aiohttp
+python3-aioresponses
 python3-apport
 python3-async-timeout
 python3-attr

subiquity/server/geoip.py

@@ -14,13 +14,12 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from abc import ABC, abstractmethod
+import aiohttp
 import logging
 import enum
-import requests
 from xml.etree import ElementTree
 
 from subiquitycore.async_helpers import (
-    run_in_thread,
     SingleInstanceTask,
 )
 
@@ -76,13 +75,14 @@ class HTTPGeoIPStrategy(GeoIPStrategy):
     """ HTTP implementation to retrieve GeoIP information. We use the
     geoip.ubuntu.com service. """
     async def get_response(self) -> str:
+        url = "https://geoip.ubuntu.com/lookup"
         try:
-            response = await run_in_thread(
+            async with aiohttp.ClientSession() as session:
-                requests.get, "https://geoip.ubuntu.com/lookup")
+                async with session.get(url) as response:
-            response.raise_for_status()
+                    response.raise_for_status()
-        except requests.exceptions.RequestException as e:
+                    return await response.text()
+        except aiohttp.ClientError as e:
             raise LookupError from e
-        return response.text
 
 
 class GeoIP:

subiquity/server/tests/test_geoip.py

@@ -13,7 +13,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import mock
+from aioresponses import aioresponses
 
 from subiquitycore.tests import SubiTestCase
 from subiquitycore.tests.mocks import make_app
@@ -47,30 +47,17 @@ empty_tz = '<Response><TimeZone></TimeZone></Response>'
 empty_cc = '<Response><CountryCode></CountryCode></Response>'
 
 
-class MockGeoIPResponse:
-    def __init__(self, text, status_code=200):
-        self.text = text
-        self.status_code = status_code
-
-    def raise_for_status(self, *args, **kwargs):
-        pass
-
-
-def requests_get_factory(text):
-    def requests_get(*args, **kwargs):
-        return MockGeoIPResponse(text)
-    return requests_get
-
-
 class TestGeoIP(SubiTestCase):
-    @mock.patch('requests.get', new=requests_get_factory(xml))
     def setUp(self):
         strategy = HTTPGeoIPStrategy()
         self.geoip = GeoIP(make_app(), strategy)
 
         async def fn():
             self.assertTrue(await self.geoip.lookup())
-        run_coro(fn())
+
+        with aioresponses() as mocked:
+            mocked.get("https://geoip.ubuntu.com/lookup", body=xml)
+            run_coro(fn())
 
     def test_countrycode(self):
         self.assertEqual("us", self.geoip.countrycode)
@@ -84,37 +71,42 @@ class TestGeoIPBadData(SubiTestCase):
         strategy = HTTPGeoIPStrategy()
         self.geoip = GeoIP(make_app(), strategy)
 
-    @mock.patch('requests.get', new=requests_get_factory(partial))
     def test_partial_reponse(self):
         async def fn():
             self.assertFalse(await self.geoip.lookup())
-        run_coro(fn())
+        with aioresponses() as mocked:
+            mocked.get("https://geoip.ubuntu.com/lookup", body=partial)
+            run_coro(fn())
 
-    @mock.patch('requests.get', new=requests_get_factory(incomplete))
     def test_incomplete(self):
         async def fn():
             self.assertFalse(await self.geoip.lookup())
-        run_coro(fn())
+        with aioresponses() as mocked:
+            mocked.get("https://geoip.ubuntu.com/lookup", body=incomplete)
+            run_coro(fn())
         self.assertIsNone(self.geoip.countrycode)
         self.assertIsNone(self.geoip.timezone)
 
-    @mock.patch('requests.get', new=requests_get_factory(long_cc))
     def test_long_cc(self):
         async def fn():
             self.assertFalse(await self.geoip.lookup())
-        run_coro(fn())
+        with aioresponses() as mocked:
+            mocked.get("https://geoip.ubuntu.com/lookup", body=long_cc)
+            run_coro(fn())
         self.assertIsNone(self.geoip.countrycode)
 
-    @mock.patch('requests.get', new=requests_get_factory(empty_cc))
     def test_empty_cc(self):
         async def fn():
             self.assertFalse(await self.geoip.lookup())
-        run_coro(fn())
+        with aioresponses() as mocked:
+            mocked.get("https://geoip.ubuntu.com/lookup", body=empty_cc)
+            run_coro(fn())
         self.assertIsNone(self.geoip.countrycode)
 
-    @mock.patch('requests.get', new=requests_get_factory(empty_tz))
     def test_empty_tz(self):
         async def fn():
             self.assertFalse(await self.geoip.lookup())
-        run_coro(fn())
+        with aioresponses() as mocked:
+            mocked.get("https://geoip.ubuntu.com/lookup", body=empty_tz)
+            run_coro(fn())
         self.assertIsNone(self.geoip.timezone)