From 0c47558729764a8c53d635c22b6f0cb967a03989 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Tue, 13 Dec 2022 10:36:19 +0100 Subject: [PATCH] convert version to named tuple Also return the new NominatimVersion rather than a string in the status result. --- nominatim/apicmd/status.py | 9 ++-- nominatim/cli.py | 2 +- nominatim/clicmd/setup.py | 4 +- nominatim/result_formatter/v1.py | 5 +-- nominatim/tools/collect_os_info.py | 6 +-- nominatim/tools/exec_utils.py | 4 +- nominatim/tools/migration.py | 21 ++++----- nominatim/tools/refresh.py | 4 +- nominatim/version.py | 57 +++++++++++++++---------- test/python/api/test_api_status.py | 10 ++--- test/python/result_formatter/test_v1.py | 6 +-- 11 files changed, 67 insertions(+), 61 deletions(-) diff --git a/nominatim/apicmd/status.py b/nominatim/apicmd/status.py index 07a1c321..85071db9 100644 --- a/nominatim/apicmd/status.py +++ b/nominatim/apicmd/status.py @@ -23,10 +23,9 @@ class StatusResult: def __init__(self, status: int, msg: str): self.status = status self.message = msg - # XXX versions really should stay tuples here - self.software_version = version.version_str() + self.software_version = version.NOMINATIM_VERSION self.data_updated: Optional[dt.datetime] = None - self.database_version: Optional[str] = None + self.database_version: Optional[version.NominatimVersion] = None async def _get_database_date(conn: AsyncConnection) -> Optional[dt.datetime]: @@ -41,13 +40,13 @@ async def _get_database_date(conn: AsyncConnection) -> Optional[dt.datetime]: return None -async def _get_database_version(conn: AsyncConnection) -> Optional[str]: +async def _get_database_version(conn: AsyncConnection) -> Optional[version.NominatimVersion]: sql = sqla.text("""SELECT value FROM nominatim_properties WHERE property = 'database_version'""") result = await conn.execute(sql) for row in result: - return cast(str, row[0]) + return version.parse_version(cast(str, row[0])) return None diff --git a/nominatim/cli.py b/nominatim/cli.py index e6214af8..cedbdb4a 100644 --- a/nominatim/cli.py +++ b/nominatim/cli.py @@ -61,7 +61,7 @@ class CommandlineParser: def nominatim_version_text(self) -> str: """ Program name and version number as string """ - text = f'Nominatim version {version.version_str()}' + text = f'Nominatim version {version.NOMINATIM_VERSION!s}' if version.GIT_COMMIT_HASH is not None: text += f' ({version.GIT_COMMIT_HASH})' return text diff --git a/nominatim/clicmd/setup.py b/nominatim/clicmd/setup.py index 68884fe1..8464e151 100644 --- a/nominatim/clicmd/setup.py +++ b/nominatim/clicmd/setup.py @@ -18,7 +18,7 @@ from nominatim.config import Configuration from nominatim.db.connection import connect from nominatim.db import status, properties from nominatim.tokenizer.base import AbstractTokenizer -from nominatim.version import version_str +from nominatim.version import NOMINATIM_VERSION from nominatim.clicmd.args import NominatimArgs from nominatim.errors import UsageError @@ -205,4 +205,4 @@ class SetupAll: except Exception as exc: # pylint: disable=broad-except LOG.error('Cannot determine date of database: %s', exc) - properties.set_property(conn, 'database_version', version_str()) + properties.set_property(conn, 'database_version', str(NOMINATIM_VERSION)) diff --git a/nominatim/result_formatter/v1.py b/nominatim/result_formatter/v1.py index 6d25ce6c..1d437af7 100644 --- a/nominatim/result_formatter/v1.py +++ b/nominatim/result_formatter/v1.py @@ -26,14 +26,13 @@ def _format_status_text(result: StatusResult) -> str: @create.format_func(StatusResult, 'json') def _format_status_json(result: StatusResult) -> str: - # XXX write a simple JSON serializer out: Dict[str, Any] = OrderedDict() out['status'] = result.status out['message'] = result.message if result.data_updated is not None: out['data_updated'] = result.data_updated.isoformat() - out['software_version'] = result.software_version + out['software_version'] = str(result.software_version) if result.database_version is not None: - out['database_version'] = result.database_version + out['database_version'] = str(result.database_version) return json.dumps(out) diff --git a/nominatim/tools/collect_os_info.py b/nominatim/tools/collect_os_info.py index 9d76f229..29e1cd53 100644 --- a/nominatim/tools/collect_os_info.py +++ b/nominatim/tools/collect_os_info.py @@ -20,7 +20,7 @@ from psycopg2.extensions import make_dsn, parse_dsn from nominatim.config import Configuration from nominatim.db.connection import connect from nominatim.typing import DictCursorResults -from nominatim.version import version_str +from nominatim.version import NOMINATIM_VERSION def convert_version(ver_tup: Tuple[int, int]) -> str: @@ -135,8 +135,8 @@ def report_system_information(config: Configuration) -> None: **Software Environment:** - Python version: {sys.version} - - Nominatim version: {version_str()} - - PostgreSQL version: {postgresql_ver} + - Nominatim version: {NOMINATIM_VERSION!s} + - PostgreSQL version: {postgresql_ver} - PostGIS version: {postgis_ver} - OS: {os_name_info()} diff --git a/nominatim/tools/exec_utils.py b/nominatim/tools/exec_utils.py index ab2ccc7c..8417f146 100644 --- a/nominatim/tools/exec_utils.py +++ b/nominatim/tools/exec_utils.py @@ -17,7 +17,7 @@ from urllib.parse import urlencode from nominatim.config import Configuration from nominatim.typing import StrPath -from nominatim.version import version_str +from nominatim.version import NOMINATIM_VERSION from nominatim.db.connection import get_pg_env LOG = logging.getLogger() @@ -162,7 +162,7 @@ def run_osm2pgsql(options: Mapping[str, Any]) -> None: def get_url(url: str) -> str: """ Get the contents from the given URL and return it as a UTF-8 string. """ - headers = {"User-Agent": f"Nominatim/{version_str()}"} + headers = {"User-Agent": f"Nominatim/{NOMINATIM_VERSION!s}"} try: request = urlrequest.Request(url, headers=headers) diff --git a/nominatim/tools/migration.py b/nominatim/tools/migration.py index 10bca15c..545f5c48 100644 --- a/nominatim/tools/migration.py +++ b/nominatim/tools/migration.py @@ -15,16 +15,14 @@ from psycopg2 import sql as pysql from nominatim.config import Configuration from nominatim.db import properties from nominatim.db.connection import connect, Connection -from nominatim.version import NOMINATIM_VERSION, version_str +from nominatim.version import NominatimVersion, NOMINATIM_VERSION, parse_version from nominatim.tools import refresh from nominatim.tokenizer import factory as tokenizer_factory from nominatim.errors import UsageError LOG = logging.getLogger() -VersionTuple = Tuple[int, int, int, int] - -_MIGRATION_FUNCTIONS : List[Tuple[VersionTuple, Callable[..., None]]] = [] +_MIGRATION_FUNCTIONS : List[Tuple[NominatimVersion, Callable[..., None]]] = [] def migrate(config: Configuration, paths: Any) -> int: """ Check for the current database version and execute migrations, @@ -37,8 +35,7 @@ def migrate(config: Configuration, paths: Any) -> int: db_version_str = None if db_version_str is not None: - parts = db_version_str.split('.') - db_version = tuple(int(x) for x in parts[:2] + parts[2].split('-')) + db_version = parse_version(db_version_str) if db_version == NOMINATIM_VERSION: LOG.warning("Database already at latest version (%s)", db_version_str) @@ -53,8 +50,7 @@ def migrate(config: Configuration, paths: Any) -> int: for version, func in _MIGRATION_FUNCTIONS: if db_version <= version: title = func.__doc__ or '' - LOG.warning("Running: %s (%s)", title.split('\n', 1)[0], - version_str(version)) + LOG.warning("Running: %s (%s)", title.split('\n', 1)[0], version) kwargs = dict(conn=conn, config=config, paths=paths) func(**kwargs) conn.commit() @@ -66,14 +62,14 @@ def migrate(config: Configuration, paths: Any) -> int: tokenizer = tokenizer_factory.get_tokenizer_for_db(config) tokenizer.update_sql_functions(config) - properties.set_property(conn, 'database_version', version_str()) + properties.set_property(conn, 'database_version', str(NOMINATIM_VERSION)) conn.commit() return 0 -def _guess_version(conn: Connection) -> VersionTuple: +def _guess_version(conn: Connection) -> NominatimVersion: """ Guess a database version when there is no property table yet. Only migrations for 3.6 and later are supported, so bail out when the version seems older. @@ -89,7 +85,7 @@ def _guess_version(conn: Connection) -> VersionTuple: 'prior to 3.6.0. Automatic migration not possible.') raise UsageError('Migration not possible.') - return (3, 5, 0, 99) + return NominatimVersion(3, 5, 0, 99) @@ -108,7 +104,8 @@ def _migration(major: int, minor: int, patch: int = 0, there. """ def decorator(func: Callable[..., None]) -> Callable[..., None]: - _MIGRATION_FUNCTIONS.append(((major, minor, patch, dbpatch), func)) + version = (NominatimVersion(major, minor, patch, dbpatch)) + _MIGRATION_FUNCTIONS.append((version, func)) return func return decorator diff --git a/nominatim/tools/refresh.py b/nominatim/tools/refresh.py index b35d3aae..45796014 100644 --- a/nominatim/tools/refresh.py +++ b/nominatim/tools/refresh.py @@ -18,7 +18,7 @@ from nominatim.config import Configuration from nominatim.db.connection import Connection, connect from nominatim.db.utils import execute_file from nominatim.db.sql_preprocessor import SQLPreprocessor -from nominatim.version import version_str +from nominatim.version import NOMINATIM_VERSION LOG = logging.getLogger() @@ -223,7 +223,7 @@ def setup_website(basedir: Path, config: Configuration, conn: Connection) -> Non @define('CONST_Debug', $_GET['debug'] ?? false); @define('CONST_LibDir', '{config.lib_dir.php}'); @define('CONST_TokenizerDir', '{config.project_dir / 'tokenizer'}'); - @define('CONST_NominatimVersion', '{version_str()}'); + @define('CONST_NominatimVersion', '{NOMINATIM_VERSION!s}'); """) diff --git a/nominatim/version.py b/nominatim/version.py index 43a30d9e..40e3bda4 100644 --- a/nominatim/version.py +++ b/nominatim/version.py @@ -7,25 +7,34 @@ """ Version information for Nominatim. """ -from typing import Optional, Tuple +from typing import Optional, NamedTuple -# Version information: major, minor, patch level, database patch level -# -# The first three numbers refer to the last released version. -# -# The database patch level tracks important changes between releases -# and must always be increased when there is a change to the database or code -# that requires a migration. -# -# When adding a migration on the development branch, raise the patch level -# to 99 to make sure that the migration is applied when updating from a -# patch release to the next minor version. Patch releases usually shouldn't -# have migrations in them. When they are needed, then make sure that the -# migration can be reapplied and set the migration version to the appropriate -# patch level when cherry-picking the commit with the migration. -# -# Released versions always have a database patch level of 0. -NOMINATIM_VERSION = (4, 2, 99, 0) +class NominatimVersion(NamedTuple): + """ Version information for Nominatim. We follow semantic versioning. + + Major, minor and patch_level refer to the last released version. + The database patch level tracks important changes between releases + and must always be increased when there is a change to the database or code + that requires a migration. + + When adding a migration on the development branch, raise the patch level + to 99 to make sure that the migration is applied when updating from a + patch release to the next minor version. Patch releases usually shouldn't + have migrations in them. When they are needed, then make sure that the + migration can be reapplied and set the migration version to the appropriate + patch level when cherry-picking the commit with the migration. + """ + + major: int + minor: int + patch_level: int + db_patch_level: int + + def __str__(self) -> str: + return f"{self.major}.{self.minor}.{self.patch_level}-{self.db_patch_level}" + + +NOMINATIM_VERSION = NominatimVersion(4, 2, 99, 0) POSTGRESQL_REQUIRED_VERSION = (9, 6) POSTGIS_REQUIRED_VERSION = (2, 2) @@ -37,9 +46,11 @@ POSTGIS_REQUIRED_VERSION = (2, 2) GIT_COMMIT_HASH : Optional[str] = None -# pylint: disable=consider-using-f-string -def version_str(version:Tuple[int, int, int, int] = NOMINATIM_VERSION) -> str: +def parse_version(version: str) -> NominatimVersion: + """ Parse a version string into a version consisting of a tuple of + four ints: major, minor, patch level, database patch level + + This is the reverse operation of `version_str()`. """ - Return a human-readable string of the version. - """ - return '{}.{}.{}-{}'.format(*version) + parts = version.split('.') + return NominatimVersion(*[int(x) for x in parts[:2] + parts[2].split('-')]) diff --git a/test/python/api/test_api_status.py b/test/python/api/test_api_status.py index 38c0fa7f..6bc1fccc 100644 --- a/test/python/api/test_api_status.py +++ b/test/python/api/test_api_status.py @@ -11,7 +11,7 @@ from pathlib import Path import datetime as dt import pytest -from nominatim.version import version_str +from nominatim.version import NOMINATIM_VERSION, NominatimVersion from nominatim.api import NominatimAPI def test_status_no_extra_info(apiobj, table_factory): @@ -24,7 +24,7 @@ def test_status_no_extra_info(apiobj, table_factory): assert result.status == 0 assert result.message == 'OK' - assert result.software_version == version_str() + assert result.software_version == NOMINATIM_VERSION assert result.database_version is None assert result.data_updated is None @@ -41,8 +41,8 @@ def test_status_full(apiobj, table_factory): assert result.status == 0 assert result.message == 'OK' - assert result.software_version == version_str() - assert result.database_version == '99.5.4-2' + assert result.software_version == NOMINATIM_VERSION + assert result.database_version == NominatimVersion(99, 5, 4, 2) assert result.data_updated == dt.datetime(2022, 12, 7, 14, 14, 46, 0, tzinfo=dt.timezone.utc) @@ -55,6 +55,6 @@ def test_status_database_not_found(monkeypatch): assert result.status == 700 assert result.message == 'Database connection failed' - assert result.software_version == version_str() + assert result.software_version == NOMINATIM_VERSION assert result.database_version is None assert result.data_updated is None diff --git a/test/python/result_formatter/test_v1.py b/test/python/result_formatter/test_v1.py index 5b7d2bfc..919f5b80 100644 --- a/test/python/result_formatter/test_v1.py +++ b/test/python/result_formatter/test_v1.py @@ -12,7 +12,7 @@ import pytest import nominatim.result_formatter.v1 as format_module from nominatim.apicmd.status import StatusResult -from nominatim.version import version_str +from nominatim.version import NOMINATIM_VERSION STATUS_FORMATS = {'text', 'json'} @@ -50,7 +50,7 @@ class TestStatusResultFormat: result = self.formatter.format(status, 'json') - assert result == '{"status": 700, "message": "Bad format.", "software_version": "%s"}' % (version_str()) + assert result == '{"status": 700, "message": "Bad format.", "software_version": "%s"}' % (NOMINATIM_VERSION, ) def test_format_json_full(self): @@ -60,4 +60,4 @@ class TestStatusResultFormat: result = self.formatter.format(status, 'json') - assert result == '{"status": 0, "message": "OK", "data_updated": "2010-02-07T20:20:03+00:00", "software_version": "%s", "database_version": "5.6"}' % (version_str()) + assert result == '{"status": 0, "message": "OK", "data_updated": "2010-02-07T20:20:03+00:00", "software_version": "%s", "database_version": "5.6"}' % (NOMINATIM_VERSION, )