Skip to content

Commit

Permalink
Merge pull request #1279 from dbungert/lp-1969393-revert-jammy
Browse files Browse the repository at this point in the history
Revert "Move to aiohttp for GeoIP requests"
  • Loading branch information
dbungert authored Apr 20, 2022
2 parents bc7ba74 + 98f2dc4 commit a80a0bd
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 27 deletions.
14 changes: 7 additions & 7 deletions subiquity/server/geoip.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@
# 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,
)

Expand Down Expand Up @@ -75,14 +76,13 @@ 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:
async with aiohttp.ClientSession() as session:
async with session.get(url) as response:
response.raise_for_status()
return await response.text()
except aiohttp.ClientError as e:
response = await run_in_thread(
requests.get, "https://geoip.ubuntu.com/lookup")
response.raise_for_status()
except requests.exceptions.RequestException as e:
raise LookupError from e
return response.text


class GeoIP:
Expand Down
48 changes: 28 additions & 20 deletions subiquity/server/tests/test_geoip.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/>.

from aioresponses import aioresponses
import mock

from subiquitycore.tests import SubiTestCase
from subiquitycore.tests.mocks import make_app
Expand Down Expand Up @@ -47,17 +47,30 @@
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())

with aioresponses() as mocked:
mocked.get("https://geoip.ubuntu.com/lookup", body=xml)
run_coro(fn())
run_coro(fn())

def test_countrycode(self):
self.assertEqual("us", self.geoip.countrycode)
Expand All @@ -71,42 +84,37 @@ def setUp(self):
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())
with aioresponses() as mocked:
mocked.get("https://geoip.ubuntu.com/lookup", body=partial)
run_coro(fn())
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())
with aioresponses() as mocked:
mocked.get("https://geoip.ubuntu.com/lookup", body=incomplete)
run_coro(fn())
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())
with aioresponses() as mocked:
mocked.get("https://geoip.ubuntu.com/lookup", body=long_cc)
run_coro(fn())
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())
with aioresponses() as mocked:
mocked.get("https://geoip.ubuntu.com/lookup", body=empty_cc)
run_coro(fn())
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())
with aioresponses() as mocked:
mocked.get("https://geoip.ubuntu.com/lookup", body=empty_tz)
run_coro(fn())
run_coro(fn())
self.assertIsNone(self.geoip.timezone)

0 comments on commit a80a0bd

Please sign in to comment.