From c19f53f194d093ede82c8765e439e98c1de52cf9 Mon Sep 17 00:00:00 2001
From: "Antoine R. Dumont (@ardumont)" <ardumont@softwareheritage.org>
Date: Fri, 3 Jun 2022 11:35:32 +0200
Subject: [PATCH] Set current_version attribute to postgresql datastore
This also simplifies the db collaborator code reusing core.db functions to check the
code version and the actual db version matches.
Related to T4305
---
requirements-swh.txt | 2 +-
swh/storage/postgresql/db.py | 27 ---------------------------
swh/storage/postgresql/storage.py | 27 ++++++++++++++++-----------
swh/storage/pytest_plugin.py | 4 ++--
swh/storage/tests/test_postgresql.py | 14 ++------------
5 files changed, 21 insertions(+), 53 deletions(-)
diff --git a/requirements-swh.txt b/requirements-swh.txt
index 336e8cf57..b9282ae08 100644
--- a/requirements-swh.txt
+++ b/requirements-swh.txt
@@ -1,4 +1,4 @@
-swh.core[db,http] >= 2
+swh.core[db,http] >= 2.9
swh.counters >= v0.8.0
swh.model >= 6.0.0
swh.objstorage >= 0.2.2
diff --git a/swh/storage/postgresql/db.py b/swh/storage/postgresql/db.py
index 969fd8e98..9edf8b94e 100644
--- a/swh/storage/postgresql/db.py
+++ b/swh/storage/postgresql/db.py
@@ -27,8 +27,6 @@ def jsonize(d):
class Db(BaseDb):
"""Proxy to the SWH DB, with wrappers around stored procedures"""
- current_version = 182
-
def mktemp_dir_entry(self, entry_type, cur=None):
self._cursor(cur).execute(
"SELECT swh_mktemp_dir_entry(%s)", (("directory_entry_%s" % entry_type),)
@@ -1543,28 +1541,3 @@ class Db(BaseDb):
row = cur.fetchone()
if row:
return row[0]
-
- dbversion_cols = ["version", "release", "description"]
-
- def dbversion(self):
- with self.transaction() as cur:
- cur.execute(
- f"""
- SELECT {', '.join(self.dbversion_cols)}
- FROM dbversion
- ORDER BY version DESC
- LIMIT 1
- """
- )
- return dict(zip(self.dbversion_cols, cur.fetchone()))
-
- def check_dbversion(self):
- dbversion = self.dbversion()["version"]
- if dbversion != self.current_version:
- logger.warning(
- "database dbversion (%s) != %s current_version (%s)",
- dbversion,
- __name__,
- self.current_version,
- )
- return dbversion == self.current_version
diff --git a/swh/storage/postgresql/storage.py b/swh/storage/postgresql/storage.py
index b83f04eed..76410e75c 100644
--- a/swh/storage/postgresql/storage.py
+++ b/swh/storage/postgresql/storage.py
@@ -9,6 +9,7 @@ import contextlib
from contextlib import contextmanager
import datetime
import itertools
+import logging
import operator
from typing import Any, Counter, Dict, Iterable, List, Optional, Sequence, Tuple
@@ -19,6 +20,7 @@ import psycopg2.pool
from swh.core.api.serializers import msgpack_dumps, msgpack_loads
from swh.core.db.common import db_transaction, db_transaction_generator
+from swh.core.db.db_utils import swh_db_version
from swh.model.hashutil import DEFAULT_ALGORITHMS, hash_to_bytes, hash_to_hex
from swh.model.model import (
SHA1_SIZE,
@@ -63,6 +65,8 @@ from swh.storage.writer import JournalWriter
from . import converters
from .db import Db
+logger = logging.getLogger(__name__)
+
# Max block size of contents to return
BULK_BLOCK_CONTENT_LEN_MAX = 10000
@@ -101,7 +105,9 @@ def convert_validation_exceptions():
class Storage:
- """SWH storage proxy, encompassing DB and object storage"""
+ """SWH storage datastore proxy, encompassing DB and object storage"""
+
+ current_version: int = 183
def __init__(
self,
@@ -193,23 +199,22 @@ class Storage:
if not self.objstorage.check_config(check_write=check_write):
return False
- if not db.check_dbversion():
+ dbversion = swh_db_version(db.conn.dsn)
+ if dbversion != self.current_version:
+ logger.warning(
+ "database dbversion (%s) != %s current_version (%s)",
+ dbversion,
+ __name__,
+ self.current_version,
+ )
return False
# Check permissions on one of the tables
- if check_write:
- check = "INSERT"
- else:
- check = "SELECT"
+ check = "INSERT" if check_write else "SELECT"
cur.execute("select has_table_privilege(current_user, 'content', %s)", (check,))
return cur.fetchone()[0]
- @db_transaction()
- def get_current_version(self, *, db: Db, cur=None):
- """Returns the current code (expected) version"""
- return db.current_version
-
def _content_unique_key(self, hash, db):
"""Given a hash (tuple or dict), return a unique key from the
aggregation of keys.
diff --git a/swh/storage/pytest_plugin.py b/swh/storage/pytest_plugin.py
index 9d4715706..c0776b78c 100644
--- a/swh/storage/pytest_plugin.py
+++ b/swh/storage/pytest_plugin.py
@@ -11,7 +11,7 @@ from pytest_postgresql import factories
from swh.core.db.pytest_plugin import initialize_database_for_module
from swh.storage import get_storage
-from swh.storage.postgresql.db import Db as StorageDb
+from swh.storage.postgresql.storage import Storage as StorageDatastore
from swh.storage.tests.storage_data import StorageData
environ["LC_ALL"] = "C.UTF-8"
@@ -22,7 +22,7 @@ swh_storage_postgresql_proc = factories.postgresql_proc(
partial(
initialize_database_for_module,
modname="storage",
- version=StorageDb.current_version,
+ version=StorageDatastore.current_version,
)
],
)
diff --git a/swh/storage/tests/test_postgresql.py b/swh/storage/tests/test_postgresql.py
index 28ec5fb58..40eb7fefc 100644
--- a/swh/storage/tests/test_postgresql.py
+++ b/swh/storage/tests/test_postgresql.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2015-2020 The Software Heritage developers
+# Copyright (C) 2015-2022 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information
@@ -12,7 +12,6 @@ import attr
import pytest
from swh.model.model import Person
-from swh.storage.postgresql.db import Db
from swh.storage.tests.storage_tests import TestStorage as _TestStorage
from swh.storage.tests.storage_tests import TestStorageGeneratedData # noqa
from swh.storage.utils import now
@@ -391,20 +390,11 @@ class TestPgStorage:
"""Calling clear buffers on real storage does nothing"""
assert swh_storage.flush() == {}
- def test_dbversion(self, swh_storage):
- with swh_storage.db() as db:
- assert db.check_dbversion()
-
- def test_dbversion_mismatch(self, swh_storage, monkeypatch):
- monkeypatch.setattr(Db, "current_version", -1)
- with swh_storage.db() as db:
- assert db.check_dbversion() is False
-
def test_check_config(self, swh_storage):
assert swh_storage.check_config(check_write=True)
assert swh_storage.check_config(check_write=False)
def test_check_config_dbversion(self, swh_storage, monkeypatch):
- monkeypatch.setattr(Db, "current_version", -1)
+ swh_storage.current_version = -1
assert swh_storage.check_config(check_write=True) is False
assert swh_storage.check_config(check_write=False) is False
--
GitLab