Merge pull request #3519 from lonvia/api-error-handling

Improve error handling around CLI api commands
This commit is contained in:
Sarah Hoffmann 2024-08-19 16:26:18 +02:00 committed by GitHub
commit 968f1cd453
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 119 additions and 113 deletions

View File

@ -38,6 +38,8 @@ class NominatimAPIAsync: #pylint: disable=too-many-instance-attributes
This class shares most of the functions with its synchronous
version. There are some additional functions or parameters,
which are documented below.
This class should usually be used as a context manager in 'with' context.
"""
def __init__(self, project_dir: Path,
environ: Optional[Mapping[str, str]] = None,
@ -166,6 +168,14 @@ class NominatimAPIAsync: #pylint: disable=too-many-instance-attributes
await self._engine.dispose()
async def __aenter__(self) -> 'NominatimAPIAsync':
return self
async def __aexit__(self, *_: Any) -> None:
await self.close()
@contextlib.asynccontextmanager
async def begin(self) -> AsyncIterator[SearchConnection]:
""" Create a new connection with automatic transaction handling.
@ -351,6 +361,8 @@ class NominatimAPI:
""" This class provides a thin synchronous wrapper around the asynchronous
Nominatim functions. It creates its own event loop and runs each
synchronous function call to completion using that loop.
This class should usually be used as a context manager in 'with' context.
"""
def __init__(self, project_dir: Path,
@ -376,10 +388,19 @@ class NominatimAPI:
This function also closes the asynchronous worker loop making
the NominatimAPI object unusable.
"""
if not self._loop.is_closed():
self._loop.run_until_complete(self._async_api.close())
self._loop.close()
def __enter__(self) -> 'NominatimAPI':
return self
def __exit__(self, *_: Any) -> None:
self.close()
@property
def config(self) -> Configuration:
""" Provide read-only access to the [configuration](Configuration.md)

View File

@ -180,7 +180,8 @@ class APISearch:
raise UsageError(f"Unsupported format '{args.format}'. "
'Use --list-formats to see supported formats.')
api = napi.NominatimAPI(args.project_dir)
try:
with napi.NominatimAPI(args.project_dir) as api:
params: Dict[str, Any] = {'max_results': args.limit + min(args.limit, 10),
'address_details': True, # needed for display name
'geometry_output': _get_geometry_output(args),
@ -203,6 +204,8 @@ class APISearch:
postalcode=args.postalcode,
country=args.country,
**params)
except napi.UsageError as ex:
raise UsageError(ex) from ex
if args.dedupe and len(results) > 1:
results = deduplicate_results(results, args.limit)
@ -260,14 +263,19 @@ class APIReverse:
if args.lat is None or args.lon is None:
raise UsageError("lat' and 'lon' parameters are required.")
api = napi.NominatimAPI(args.project_dir)
layers = _get_layers(args, napi.DataLayer.ADDRESS | napi.DataLayer.POI)
try:
with napi.NominatimAPI(args.project_dir) as api:
result = api.reverse(napi.Point(args.lon, args.lat),
max_rank=zoom_to_rank(args.zoom or 18),
layers=_get_layers(args, napi.DataLayer.ADDRESS | napi.DataLayer.POI),
layers=layers,
address_details=True, # needed for display name
geometry_output=_get_geometry_output(args),
geometry_simplification=args.polygon_threshold,
locales=_get_locales(args, api.config.DEFAULT_LANGUAGE))
except napi.UsageError as ex:
raise UsageError(ex) from ex
if args.format == 'debug':
print(loglib.get_and_disable())
@ -323,12 +331,15 @@ class APILookup:
places = [napi.OsmID(o[0], int(o[1:])) for o in args.ids]
api = napi.NominatimAPI(args.project_dir)
try:
with napi.NominatimAPI(args.project_dir) as api:
results = api.lookup(places,
address_details=True, # needed for display name
geometry_output=_get_geometry_output(args),
geometry_simplification=args.polygon_threshold or 0.0,
locales=_get_locales(args, api.config.DEFAULT_LANGUAGE))
except napi.UsageError as ex:
raise UsageError(ex) from ex
if args.format == 'debug':
print(loglib.get_and_disable())
@ -410,7 +421,8 @@ class APIDetails:
raise UsageError('One of the arguments --node/-n --way/-w '
'--relation/-r --place_id/-p is required/')
api = napi.NominatimAPI(args.project_dir)
try:
with napi.NominatimAPI(args.project_dir) as api:
locales = _get_locales(args, api.config.DEFAULT_LANGUAGE)
result = api.details(place,
address_details=args.addressdetails,
@ -421,6 +433,8 @@ class APIDetails:
if args.polygon_geojson
else napi.GeometryFormat.NONE,
locales=locales)
except napi.UsageError as ex:
raise UsageError(ex) from ex
if args.format == 'debug':
print(loglib.get_and_disable())
@ -465,7 +479,11 @@ class APIStatus:
raise UsageError(f"Unsupported format '{args.format}'. "
'Use --list-formats to see supported formats.')
status = napi.NominatimAPI(args.project_dir).status()
try:
with napi.NominatimAPI(args.project_dir) as api:
status = api.status()
except napi.UsageError as ex:
raise UsageError(ex) from ex
if args.format == 'debug':
print(loglib.get_and_disable())

View File

@ -9,6 +9,7 @@ Helper fixtures for API call tests.
"""
from pathlib import Path
import pytest
import pytest_asyncio
import time
import datetime as dt
@ -244,3 +245,9 @@ def frontend(request, event_loop, tmp_path):
for api in testapis:
api.close()
@pytest_asyncio.fixture
async def api(temp_db):
async with napi.NominatimAPIAsync(Path('/invalid')) as api:
yield api

View File

@ -40,10 +40,9 @@ async def conn(table_factory):
table_factory('word',
definition='word_id INT, word_token TEXT, type TEXT, word TEXT, info JSONB')
api = NominatimAPIAsync(Path('/invalid'), {})
async with NominatimAPIAsync(Path('/invalid'), {}) as api:
async with api.begin() as conn:
yield conn
await api.close()
@pytest.mark.asyncio

View File

@ -74,10 +74,9 @@ async def conn(table_factory, temp_db_cursor):
temp_db_cursor.execute("""CREATE OR REPLACE FUNCTION make_standard_name(name TEXT)
RETURNS TEXT AS $$ SELECT lower(name); $$ LANGUAGE SQL;""")
api = NominatimAPIAsync(Path('/invalid'), {})
async with NominatimAPIAsync(Path('/invalid'), {}) as api:
async with api.begin() as conn:
yield conn
await api.close()
@pytest.mark.asyncio

View File

@ -11,41 +11,35 @@ from pathlib import Path
import pytest
from nominatim_api import NominatimAPIAsync
from nominatim_api.search.query_analyzer_factory import make_query_analyzer
from nominatim_api.search.icu_tokenizer import ICUQueryAnalyzer
@pytest.mark.asyncio
async def test_import_icu_tokenizer(table_factory):
async def test_import_icu_tokenizer(table_factory, api):
table_factory('nominatim_properties',
definition='property TEXT, value TEXT',
content=(('tokenizer', 'icu'),
('tokenizer_import_normalisation', ':: lower();'),
('tokenizer_import_transliteration', "'1' > '/1/'; 'ä' > 'ä '")))
api = NominatimAPIAsync(Path('/invalid'), {})
async with api.begin() as conn:
ana = await make_query_analyzer(conn)
assert isinstance(ana, ICUQueryAnalyzer)
await api.close()
@pytest.mark.asyncio
async def test_import_missing_property(table_factory):
api = NominatimAPIAsync(Path('/invalid'), {})
async def test_import_missing_property(table_factory, api):
table_factory('nominatim_properties',
definition='property TEXT, value TEXT')
async with api.begin() as conn:
with pytest.raises(ValueError, match='Property.*not found'):
await make_query_analyzer(conn)
await api.close()
@pytest.mark.asyncio
async def test_import_missing_module(table_factory):
api = NominatimAPIAsync(Path('/invalid'), {})
async def test_import_missing_module(table_factory, api):
table_factory('nominatim_properties',
definition='property TEXT, value TEXT',
content=(('tokenizer', 'missing'),))
@ -53,5 +47,3 @@ async def test_import_missing_module(table_factory):
async with api.begin() as conn:
with pytest.raises(RuntimeError, match='Tokenizer not found'):
await make_query_analyzer(conn)
await api.close()

View File

@ -9,45 +9,34 @@ Tests for enhanced connection class for API functions.
"""
from pathlib import Path
import pytest
import pytest_asyncio
import sqlalchemy as sa
from nominatim_api import NominatimAPIAsync
@pytest_asyncio.fixture
async def apiobj(temp_db):
""" Create an asynchronous SQLAlchemy engine for the test DB.
"""
api = NominatimAPIAsync(Path('/invalid'), {})
yield api
await api.close()
@pytest.mark.asyncio
async def test_run_scalar(apiobj, table_factory):
async def test_run_scalar(api, table_factory):
table_factory('foo', definition='that TEXT', content=(('a', ),))
async with apiobj.begin() as conn:
async with api.begin() as conn:
assert await conn.scalar(sa.text('SELECT * FROM foo')) == 'a'
@pytest.mark.asyncio
async def test_run_execute(apiobj, table_factory):
async def test_run_execute(api, table_factory):
table_factory('foo', definition='that TEXT', content=(('a', ),))
async with apiobj.begin() as conn:
async with api.begin() as conn:
result = await conn.execute(sa.text('SELECT * FROM foo'))
assert result.fetchone()[0] == 'a'
@pytest.mark.asyncio
async def test_get_property_existing_cached(apiobj, table_factory):
async def test_get_property_existing_cached(api, table_factory):
table_factory('nominatim_properties',
definition='property TEXT, value TEXT',
content=(('dbv', '96723'), ))
async with apiobj.begin() as conn:
async with api.begin() as conn:
assert await conn.get_property('dbv') == '96723'
await conn.execute(sa.text('TRUNCATE nominatim_properties'))
@ -56,12 +45,12 @@ async def test_get_property_existing_cached(apiobj, table_factory):
@pytest.mark.asyncio
async def test_get_property_existing_uncached(apiobj, table_factory):
async def test_get_property_existing_uncached(api, table_factory):
table_factory('nominatim_properties',
definition='property TEXT, value TEXT',
content=(('dbv', '96723'), ))
async with apiobj.begin() as conn:
async with api.begin() as conn:
assert await conn.get_property('dbv') == '96723'
await conn.execute(sa.text("UPDATE nominatim_properties SET value = '1'"))
@ -71,23 +60,23 @@ async def test_get_property_existing_uncached(apiobj, table_factory):
@pytest.mark.asyncio
@pytest.mark.parametrize('param', ['foo', 'DB:server_version'])
async def test_get_property_missing(apiobj, table_factory, param):
async def test_get_property_missing(api, table_factory, param):
table_factory('nominatim_properties',
definition='property TEXT, value TEXT')
async with apiobj.begin() as conn:
async with api.begin() as conn:
with pytest.raises(ValueError):
await conn.get_property(param)
@pytest.mark.asyncio
async def test_get_db_property_existing(apiobj):
async with apiobj.begin() as conn:
async def test_get_db_property_existing(api):
async with api.begin() as conn:
assert await conn.get_db_property('server_version') > 0
@pytest.mark.asyncio
async def test_get_db_property_existing(apiobj):
async with apiobj.begin() as conn:
async def test_get_db_property_existing(api):
async with api.begin() as conn:
with pytest.raises(ValueError):
await conn.get_db_property('dfkgjd.rijg')

View File

@ -11,19 +11,10 @@ import json
from pathlib import Path
import pytest
import pytest_asyncio
from fake_adaptor import FakeAdaptor, FakeError, FakeResponse
import nominatim_api.v1.server_glue as glue
import nominatim_api as napi
@pytest_asyncio.fixture
async def api():
api = napi.NominatimAPIAsync(Path('/invalid'))
yield api
await api.close()
class TestDeletableEndPoint:
@ -61,4 +52,3 @@ class TestDeletableEndPoint:
{'place_id': 3, 'country_code': 'cd', 'name': None,
'osm_id': 781, 'osm_type': 'R',
'class': 'landcover', 'type': 'grass'}]

View File

@ -12,19 +12,10 @@ import datetime as dt
from pathlib import Path
import pytest
import pytest_asyncio
from fake_adaptor import FakeAdaptor, FakeError, FakeResponse
import nominatim_api.v1.server_glue as glue
import nominatim_api as napi
@pytest_asyncio.fixture
async def api():
api = napi.NominatimAPIAsync(Path('/invalid'))
yield api
await api.close()
class TestPolygonsEndPoint: