From 1b8b58dbd91ef87246d4ad26b22ef659a7250635 Mon Sep 17 00:00:00 2001 From: Alessio Siniscalchi Date: Mon, 25 Sep 2023 10:53:45 +0200 Subject: [PATCH 1/4] available new config variables for ro connection string --- cads_catalogue/config.py | 30 +++++++++++++++++++++++ tests/test_01_config.py | 53 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/cads_catalogue/config.py b/cads_catalogue/config.py index fac63e2..53deed9 100644 --- a/cads_catalogue/config.py +++ b/cads_catalogue/config.py @@ -36,10 +36,15 @@ class SqlalchemySettings: - ``catalogue_db_name``: database name. """ + ro_catalogue_db_password: str = dataclasses.field(repr=False) + ro_catalogue_db_user: str + ro_catalogue_db_host: str + ro_catalogue_db_name: str catalogue_db_password: str = dataclasses.field(repr=False) catalogue_db_user: str = "catalogue" catalogue_db_host: str = "catalogue-db" catalogue_db_name: str = "catalogue" + pool_recycle: int = 60 def __init__(self, **kwargs): @@ -75,6 +80,22 @@ def __post_init__(self): f"{field.name} '{value}' has not type {repr(field.type)}" ) + # defaults of ro_* fields + default_fields_map = { + "ro_catalogue_db_user": "catalogue_db_user", + "ro_catalogue_db_password": "catalogue_db_password", + "ro_catalogue_db_host": "catalogue_db_host", + "ro_catalogue_db_name": "catalogue_db_name", + } + for field in dataclasses.fields(self): + value = getattr(self, field.name) + if field.name in default_fields_map and value in [ + dataclasses.MISSING, + None, + ]: + default_value = getattr(self, default_fields_map[field.name]) + setattr(self, field.name, default_value) + # validations # defined fields without a default must have a value for field in dataclasses.fields(self): @@ -94,6 +115,15 @@ def connection_string(self) -> str: f"/{self.catalogue_db_name}" ) + @property + def connection_string_ro(self) -> str: + """Create reader psql connection string in read-only mode.""" + return ( + f"postgresql://{self.ro_catalogue_db_user}" + f":{self.ro_catalogue_db_password}@{self.ro_catalogue_db_host}" + f"/{self.ro_catalogue_db_name}" + ) + @dataclasses.dataclass(kw_only=True) class ObjectStorageSettings: diff --git a/tests/test_01_config.py b/tests/test_01_config.py index 90841d5..def94fb 100644 --- a/tests/test_01_config.py +++ b/tests/test_01_config.py @@ -6,42 +6,72 @@ from cads_catalogue import config -def test_sqlalchemysettings(temp_environ: Any) -> None: +def test_sqlalchemysettings_1(temp_environ: Any) -> None: # check settings must have a password set (no default) temp_environ.pop("catalogue_db_password", default=None) with pytest.raises(ValueError) as excinfo: config.SqlalchemySettings() assert "catalogue_db_password" in str(excinfo.value) - config.dbsettings = None + +def test_sqlalchemysettings_2(temp_environ: Any) -> None: # also an empty password can be set settings = config.SqlalchemySettings(catalogue_db_password="") assert settings.catalogue_db_password == "" - config.dbsettings = None + +def test_sqlalchemysettings_3(temp_environ: Any) -> None: # also a not empty password can be set temp_environ["catalogue_db_password"] = "a password" settings = config.SqlalchemySettings() assert settings.catalogue_db_password == "a password" - config.dbsettings = None + +def test_sqlalchemysettings_4(temp_environ: Any) -> None: # take also other values from the environment temp_environ["catalogue_db_password"] = "1" temp_environ["catalogue_db_user"] = "2" temp_environ["catalogue_db_host"] = "3" temp_environ["catalogue_db_name"] = "4" + temp_environ["ro_catalogue_db_password"] = "5" + temp_environ["ro_catalogue_db_user"] = "6" + temp_environ["ro_catalogue_db_host"] = "7" + temp_environ["ro_catalogue_db_name"] = "8" + temp_environ["pool_recycle"] = "9" + settings = config.SqlalchemySettings() + assert settings.catalogue_db_password == "1" + assert settings.catalogue_db_user == "2" + assert settings.catalogue_db_host == "3" + assert settings.catalogue_db_name == "4" + assert settings.ro_catalogue_db_password == "5" + assert settings.ro_catalogue_db_user == "6" + assert settings.ro_catalogue_db_host == "7" + assert settings.ro_catalogue_db_name == "8" + assert settings.pool_recycle == 9 + + +def test_sqlalchemysettings_5(temp_environ: Any) -> None: + # check defaults for read-only fields + temp_environ["catalogue_db_password"] = "1" + temp_environ["catalogue_db_user"] = "2" + temp_environ["catalogue_db_host"] = "3" + temp_environ["catalogue_db_name"] = "4" temp_environ["pool_recycle"] = "5" settings = config.SqlalchemySettings() assert settings.catalogue_db_password == "1" assert settings.catalogue_db_user == "2" assert settings.catalogue_db_host == "3" assert settings.catalogue_db_name == "4" + assert settings.ro_catalogue_db_password == "1" + assert settings.ro_catalogue_db_user == "2" + assert settings.ro_catalogue_db_host == "3" + assert settings.ro_catalogue_db_name == "4" assert settings.pool_recycle == 5 def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> None: temp_environ["catalogue_db_password"] = "apassword" - + temp_environ["ro_catalogue_db_password"] = "ro_apassword" # initially global settings is importable, but it is None assert config.dbsettings is None @@ -51,6 +81,10 @@ def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> effective_settings.connection_string == "postgresql://catalogue:apassword@catalogue-db/catalogue" ) + assert ( + effective_settings.connection_string_ro + == "postgresql://catalogue:ro_apassword@catalogue-db/catalogue" + ) assert config.dbsettings == effective_settings config.dbsettings = None @@ -60,17 +94,26 @@ def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> "catalogue_db_password": "secret1", "catalogue_db_host": "myhost", "catalogue_db_name": "mycatalogue", + "ro_catalogue_db_user": "monica2", + "ro_catalogue_db_password": "secret2", + "ro_catalogue_db_host": "myhost2", + "ro_catalogue_db_name": "mycatalogue2", } my_settings_connection_string = ( "postgresql://%(catalogue_db_user)s:%(catalogue_db_password)s" "@%(catalogue_db_host)s/%(catalogue_db_name)s" % my_settings_dict ) + my_settings_connection_string_ro = ( + "postgresql://%(ro_catalogue_db_user)s:%(ro_catalogue_db_password)s" + "@%(ro_catalogue_db_host)s/%(ro_catalogue_db_name)s" % my_settings_dict + ) mysettings = config.SqlalchemySettings(**my_settings_dict) # type: ignore effective_settings = config.ensure_settings(mysettings) assert config.dbsettings == effective_settings assert effective_settings == mysettings assert effective_settings.connection_string == my_settings_connection_string + assert effective_settings.connection_string_ro == my_settings_connection_string_ro config.dbsettings = None From fb37ca00c8a83955c7632d105ec7e9b4bfffe67c Mon Sep 17 00:00:00 2001 From: Alessio Siniscalchi Date: Mon, 25 Sep 2023 11:50:38 +0200 Subject: [PATCH 2/4] use of multiple connections --- cads_catalogue/database.py | 20 ++++++++++++-------- cads_catalogue/faceted_search.py | 2 +- tests/test_02_database.py | 27 +++++++++++++++++---------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/cads_catalogue/database.py b/cads_catalogue/database.py index f58c99b..03e771d 100644 --- a/cads_catalogue/database.py +++ b/cads_catalogue/database.py @@ -279,24 +279,28 @@ class Licence(BaseModel): ) -def ensure_session_obj(session_obj: sa.orm.sessionmaker | None) -> sa.orm.sessionmaker: - """If `session_obj` is None, create a new session object. +def ensure_session_obj(read_only: bool = False) -> sa.orm.sessionmaker: + """Create a new session object bound to the catalogue database. Parameters ---------- - session_obj: sqlalchemy Session object + read_only: if True, return the sessionmaker object for read-only sessions (default False). Returns ------- session_obj: a SQLAlchemy Session object """ - if session_obj: - return session_obj settings = config.ensure_settings(config.dbsettings) - session_obj = sa.orm.sessionmaker( - sa.create_engine(settings.connection_string, pool_recycle=settings.pool_recycle) - ) + if read_only: + engine = sa.create_engine( + settings.connection_string_ro, pool_recycle=settings.pool_recycle + ) + else: + engine = sa.create_engine( + settings.connection_string, pool_recycle=settings.pool_recycle + ) + session_obj = sa.orm.sessionmaker(engine) return session_obj diff --git a/cads_catalogue/faceted_search.py b/cads_catalogue/faceted_search.py index 72eb8bf..f86f3e7 100644 --- a/cads_catalogue/faceted_search.py +++ b/cads_catalogue/faceted_search.py @@ -27,7 +27,7 @@ from cads_catalogue import database from cads_catalogue.faceted_search import get_datasets_by_keywords, get_faceted_stats -session_obj = database.ensure_session_obj() +session_obj = database.ensure_session_obj(read_only=True) session = session_obj() # consider all the datasets (but you can start with a filtered set of resources, diff --git a/tests/test_02_database.py b/tests/test_02_database.py index 4c5735f..8c7003a 100644 --- a/tests/test_02_database.py +++ b/tests/test_02_database.py @@ -23,18 +23,25 @@ def test_init_database(postgresql: Connection[str]) -> None: database.init_database(connection_string, force=True) assert set(conn.execute(query).scalars()) == expected_tables_complete # type: ignore + conn.close() -def test_ensure_session_obj( - postgresql: Connection[str], session_obj: sessionmaker, temp_environ: Any -) -> None: - # case of session is already set - ret_value = database.ensure_session_obj(session_obj) - assert ret_value is session_obj - config.dbsettings = None - - # case of session not set +def test_ensure_session_obj(postgresql: Connection[str], temp_environ: Any) -> None: + temp_environ["catalogue_db_host"] = "cataloguehost" temp_environ["catalogue_db_password"] = postgresql.info.password - ret_value = database.ensure_session_obj(None) + temp_environ["ro_catalogue_db_host"] = "readonlyhost" + ret_value = database.ensure_session_obj() assert isinstance(ret_value, sessionmaker) + assert ( + str(ret_value.kw["bind"].url) + == "postgresql://catalogue:***@cataloguehost/catalogue" + ) + + config.dbsettings = None + ret_value = database.ensure_session_obj(read_only=True) + assert isinstance(ret_value, sessionmaker) + assert ( + str(ret_value.kw["bind"].url) + == "postgresql://catalogue:***@readonlyhost/catalogue" + ) config.dbsettings = None From 19b35a2c9aa5343447c16f7e09cbce2d7f1255c2 Mon Sep 17 00:00:00 2001 From: Alessio Siniscalchi Date: Mon, 25 Sep 2023 16:19:47 +0200 Subject: [PATCH 3/4] use right env names + optimize validator --- cads_catalogue/config.py | 56 ++++++++--------- tests/test_01_config.py | 125 +++++++++++++++++++------------------- tests/test_02_database.py | 8 ++- 3 files changed, 95 insertions(+), 94 deletions(-) diff --git a/cads_catalogue/config.py b/cads_catalogue/config.py index 53deed9..7701d9c 100644 --- a/cads_catalogue/config.py +++ b/cads_catalogue/config.py @@ -36,14 +36,16 @@ class SqlalchemySettings: - ``catalogue_db_name``: database name. """ - ro_catalogue_db_password: str = dataclasses.field(repr=False) - ro_catalogue_db_user: str - ro_catalogue_db_host: str - ro_catalogue_db_name: str - catalogue_db_password: str = dataclasses.field(repr=False) - catalogue_db_user: str = "catalogue" - catalogue_db_host: str = "catalogue-db" - catalogue_db_name: str = "catalogue" + read_db_user: str | None + read_db_password: str | None + write_db_user: str | None + write_db_password: str | None + db_host: str | None + + catalogue_db_user: str | None = None + catalogue_db_password: str | None = None + catalogue_db_host: str | None = None + catalogue_db_name: str | None = None pool_recycle: int = 60 @@ -80,12 +82,13 @@ def __post_init__(self): f"{field.name} '{value}' has not type {repr(field.type)}" ) - # defaults of ro_* fields + # defaults for backward-compatibility fields default_fields_map = { - "ro_catalogue_db_user": "catalogue_db_user", - "ro_catalogue_db_password": "catalogue_db_password", - "ro_catalogue_db_host": "catalogue_db_host", - "ro_catalogue_db_name": "catalogue_db_name", + "read_db_user": "catalogue_db_user", + "write_db_user": "catalogue_db_user", + "read_db_password": "catalogue_db_password", + "write_db_password": "catalogue_db_password", + "db_host": "catalogue_db_host", } for field in dataclasses.fields(self): value = getattr(self, field.name) @@ -100,29 +103,23 @@ def __post_init__(self): # defined fields without a default must have a value for field in dataclasses.fields(self): value = getattr(self, field.name) - if field.default == dataclasses.MISSING and value == dataclasses.MISSING: + if field.default == dataclasses.MISSING and value in ( + dataclasses.MISSING, + None, + ): raise ValueError(f"{field.name} must be set") - # catalogue_db_password must be set - if self.catalogue_db_password is None: - raise ValueError("catalogue_db_password must be set") @property def connection_string(self) -> str: """Create reader psql connection string.""" - return ( - f"postgresql://{self.catalogue_db_user}" - f":{self.catalogue_db_password}@{self.catalogue_db_host}" - f"/{self.catalogue_db_name}" - ) + url = f"postgresql://{self.write_db_user}:{self.write_db_password}@{self.db_host}/{self.catalogue_db_name}" + return url @property def connection_string_ro(self) -> str: """Create reader psql connection string in read-only mode.""" - return ( - f"postgresql://{self.ro_catalogue_db_user}" - f":{self.ro_catalogue_db_password}@{self.ro_catalogue_db_host}" - f"/{self.ro_catalogue_db_name}" - ) + url = f"postgresql://{self.read_db_user}:{self.read_db_password}@{self.db_host}/{self.catalogue_db_name}" + return url @dataclasses.dataclass(kw_only=True) @@ -179,7 +176,10 @@ def __post_init__(self): # defined fields without a default must have a value for field in dataclasses.fields(self): value = getattr(self, field.name) - if field.default == dataclasses.MISSING and value == dataclasses.MISSING: + if field.default == dataclasses.MISSING and value in ( + dataclasses.MISSING, + None, + ): raise ValueError(f"{field.name} must be set") # storage_password must be set if self.storage_password is None: diff --git a/tests/test_01_config.py b/tests/test_01_config.py index def94fb..8123021 100644 --- a/tests/test_01_config.py +++ b/tests/test_01_config.py @@ -6,72 +6,70 @@ from cads_catalogue import config -def test_sqlalchemysettings_1(temp_environ: Any) -> None: +def test_sqlalchemysettings(temp_environ: Any) -> None: # check settings must have a password set (no default) temp_environ.pop("catalogue_db_password", default=None) with pytest.raises(ValueError) as excinfo: config.SqlalchemySettings() - assert "catalogue_db_password" in str(excinfo.value) + assert "read_db_user" in str(excinfo.value) - -def test_sqlalchemysettings_2(temp_environ: Any) -> None: # also an empty password can be set + temp_environ["catalogue_db_user"] = "user1" + temp_environ["catalogue_db_host"] = "host1" + temp_environ["catalogue_db_name"] = "dbname1" settings = config.SqlalchemySettings(catalogue_db_password="") assert settings.catalogue_db_password == "" + # check backward compatibility defaults + assert settings.__dict__ == { + "catalogue_db_user": "user1", + "catalogue_db_password": "", + "catalogue_db_host": "host1", + "catalogue_db_name": "dbname1", + "read_db_user": "user1", + "read_db_password": "", + "write_db_user": "user1", + "write_db_password": "", + "db_host": "host1", + "pool_recycle": 60, + "match_args": {"catalogue_db_password": ""}, + } -def test_sqlalchemysettings_3(temp_environ: Any) -> None: - # also a not empty password can be set - temp_environ["catalogue_db_password"] = "a password" - settings = config.SqlalchemySettings() - assert settings.catalogue_db_password == "a password" - - -def test_sqlalchemysettings_4(temp_environ: Any) -> None: - # take also other values from the environment - temp_environ["catalogue_db_password"] = "1" - temp_environ["catalogue_db_user"] = "2" - temp_environ["catalogue_db_host"] = "3" - temp_environ["catalogue_db_name"] = "4" - temp_environ["ro_catalogue_db_password"] = "5" - temp_environ["ro_catalogue_db_user"] = "6" - temp_environ["ro_catalogue_db_host"] = "7" - temp_environ["ro_catalogue_db_name"] = "8" - temp_environ["pool_recycle"] = "9" + # use not backward compatibility variables + temp_environ["read_db_user"] = "ro_user" + temp_environ["read_db_password"] = "ro_password" + temp_environ["write_db_user"] = "rw_user" + temp_environ["write_db_password"] = "rw_password" + temp_environ["db_host"] = "new_db_host" settings = config.SqlalchemySettings() - assert settings.catalogue_db_password == "1" - assert settings.catalogue_db_user == "2" - assert settings.catalogue_db_host == "3" - assert settings.catalogue_db_name == "4" - assert settings.ro_catalogue_db_password == "5" - assert settings.ro_catalogue_db_user == "6" - assert settings.ro_catalogue_db_host == "7" - assert settings.ro_catalogue_db_name == "8" - assert settings.pool_recycle == 9 - - -def test_sqlalchemysettings_5(temp_environ: Any) -> None: - # check defaults for read-only fields - temp_environ["catalogue_db_password"] = "1" - temp_environ["catalogue_db_user"] = "2" - temp_environ["catalogue_db_host"] = "3" - temp_environ["catalogue_db_name"] = "4" - temp_environ["pool_recycle"] = "5" - settings = config.SqlalchemySettings() - assert settings.catalogue_db_password == "1" - assert settings.catalogue_db_user == "2" - assert settings.catalogue_db_host == "3" - assert settings.catalogue_db_name == "4" - assert settings.ro_catalogue_db_password == "1" - assert settings.ro_catalogue_db_user == "2" - assert settings.ro_catalogue_db_host == "3" - assert settings.ro_catalogue_db_name == "4" - assert settings.pool_recycle == 5 + assert settings.__dict__ == { + "catalogue_db_user": "user1", + "catalogue_db_password": None, + "catalogue_db_host": "host1", + "catalogue_db_name": "dbname1", + "read_db_user": "ro_user", + "read_db_password": "ro_password", + "write_db_user": "rw_user", + "write_db_password": "rw_password", + "db_host": "new_db_host", + "pool_recycle": 60, + "match_args": {}, + } + assert ( + settings.connection_string + == "postgresql://rw_user:rw_password@new_db_host/dbname1" + ) + assert ( + settings.connection_string_ro + == "postgresql://ro_user:ro_password@new_db_host/dbname1" + ) def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> None: + temp_environ["catalogue_db_user"] = "auser" temp_environ["catalogue_db_password"] = "apassword" - temp_environ["ro_catalogue_db_password"] = "ro_apassword" + temp_environ["catalogue_db_host"] = "ahost" + temp_environ["catalogue_db_name"] = "aname" # initially global settings is importable, but it is None assert config.dbsettings is None @@ -79,11 +77,11 @@ def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> effective_settings = config.ensure_settings() assert ( effective_settings.connection_string - == "postgresql://catalogue:apassword@catalogue-db/catalogue" + == "postgresql://auser:apassword@ahost/aname" ) assert ( effective_settings.connection_string_ro - == "postgresql://catalogue:ro_apassword@catalogue-db/catalogue" + == "postgresql://auser:apassword@ahost/aname" ) assert config.dbsettings == effective_settings config.dbsettings = None @@ -93,21 +91,22 @@ def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> "catalogue_db_user": "monica", "catalogue_db_password": "secret1", "catalogue_db_host": "myhost", - "catalogue_db_name": "mycatalogue", - "ro_catalogue_db_user": "monica2", - "ro_catalogue_db_password": "secret2", - "ro_catalogue_db_host": "myhost2", - "ro_catalogue_db_name": "mycatalogue2", + "catalogue_db_name": "mybroker", + "read_db_user": "ro_user", + "read_db_password": "ro_password", + "write_db_user": "rw_user", + "write_db_password": "rw_password", + "db_host": "new_db_host", } my_settings_connection_string = ( - "postgresql://%(catalogue_db_user)s:%(catalogue_db_password)s" - "@%(catalogue_db_host)s/%(catalogue_db_name)s" % my_settings_dict + "postgresql://%(write_db_user)s:%(write_db_password)s" + "@%(db_host)s/%(catalogue_db_name)s" % my_settings_dict ) my_settings_connection_string_ro = ( - "postgresql://%(ro_catalogue_db_user)s:%(ro_catalogue_db_password)s" - "@%(ro_catalogue_db_host)s/%(ro_catalogue_db_name)s" % my_settings_dict + "postgresql://%(read_db_user)s:%(read_db_password)s" + "@%(db_host)s/%(catalogue_db_name)s" % my_settings_dict ) - mysettings = config.SqlalchemySettings(**my_settings_dict) # type: ignore + mysettings = config.SqlalchemySettings(**my_settings_dict) effective_settings = config.ensure_settings(mysettings) assert config.dbsettings == effective_settings diff --git a/tests/test_02_database.py b/tests/test_02_database.py index 8c7003a..999e00f 100644 --- a/tests/test_02_database.py +++ b/tests/test_02_database.py @@ -29,12 +29,14 @@ def test_init_database(postgresql: Connection[str]) -> None: def test_ensure_session_obj(postgresql: Connection[str], temp_environ: Any) -> None: temp_environ["catalogue_db_host"] = "cataloguehost" temp_environ["catalogue_db_password"] = postgresql.info.password - temp_environ["ro_catalogue_db_host"] = "readonlyhost" + temp_environ["read_db_user"] = "readonlyuser" + temp_environ["write_db_user"] = "writeuser" + temp_environ["catalogue_db_name"] = "catalogue" ret_value = database.ensure_session_obj() assert isinstance(ret_value, sessionmaker) assert ( str(ret_value.kw["bind"].url) - == "postgresql://catalogue:***@cataloguehost/catalogue" + == "postgresql://writeuser:***@cataloguehost/catalogue" ) config.dbsettings = None @@ -42,6 +44,6 @@ def test_ensure_session_obj(postgresql: Connection[str], temp_environ: Any) -> N assert isinstance(ret_value, sessionmaker) assert ( str(ret_value.kw["bind"].url) - == "postgresql://catalogue:***@readonlyhost/catalogue" + == "postgresql://readonlyuser:***@cataloguehost/catalogue" ) config.dbsettings = None From eac204900e98860190e674f7f55fed23c93a5030 Mon Sep 17 00:00:00 2001 From: Alessio Siniscalchi Date: Tue, 17 Oct 2023 18:15:39 +0200 Subject: [PATCH 4/4] refactoring according to recent agreements --- cads_catalogue/config.py | 51 ++++---------------- cads_catalogue/database.py | 2 +- tests/test_01_config.py | 98 ++++++++++++++++---------------------- tests/test_02_database.py | 9 ++-- 4 files changed, 55 insertions(+), 105 deletions(-) diff --git a/cads_catalogue/config.py b/cads_catalogue/config.py index 7701d9c..a8c8740 100644 --- a/cads_catalogue/config.py +++ b/cads_catalogue/config.py @@ -32,21 +32,16 @@ class SqlalchemySettings: - ``catalogue_db_user``: postgres username. - ``catalogue_db_password``: postgres password. - - ``catalogue_db_host``: hostname for the connection. + - ``catalogue_db_host``: hostname for r/w connection. + - ``catalogue_db_host``: hostname for read-only connection. - ``catalogue_db_name``: database name. """ - read_db_user: str | None - read_db_password: str | None - write_db_user: str | None - write_db_password: str | None - db_host: str | None - catalogue_db_user: str | None = None catalogue_db_password: str | None = None catalogue_db_host: str | None = None + catalogue_db_host_read: str | None = None catalogue_db_name: str | None = None - pool_recycle: int = 60 def __init__(self, **kwargs): @@ -82,43 +77,23 @@ def __post_init__(self): f"{field.name} '{value}' has not type {repr(field.type)}" ) - # defaults for backward-compatibility fields - default_fields_map = { - "read_db_user": "catalogue_db_user", - "write_db_user": "catalogue_db_user", - "read_db_password": "catalogue_db_password", - "write_db_password": "catalogue_db_password", - "db_host": "catalogue_db_host", - } - for field in dataclasses.fields(self): - value = getattr(self, field.name) - if field.name in default_fields_map and value in [ - dataclasses.MISSING, - None, - ]: - default_value = getattr(self, default_fields_map[field.name]) - setattr(self, field.name, default_value) - # validations - # defined fields without a default must have a value + # defined fields must have a not None value for field in dataclasses.fields(self): value = getattr(self, field.name) - if field.default == dataclasses.MISSING and value in ( - dataclasses.MISSING, - None, - ): + if value in (dataclasses.MISSING, None): raise ValueError(f"{field.name} must be set") @property def connection_string(self) -> str: """Create reader psql connection string.""" - url = f"postgresql://{self.write_db_user}:{self.write_db_password}@{self.db_host}/{self.catalogue_db_name}" + url = f"postgresql://{self.catalogue_db_user}:{self.catalogue_db_password}@{self.catalogue_db_host}/{self.catalogue_db_name}" return url @property - def connection_string_ro(self) -> str: + def connection_string_read(self) -> str: """Create reader psql connection string in read-only mode.""" - url = f"postgresql://{self.read_db_user}:{self.read_db_password}@{self.db_host}/{self.catalogue_db_name}" + url = f"postgresql://{self.catalogue_db_user}:{self.catalogue_db_password}@{self.catalogue_db_host_read}/{self.catalogue_db_name}" return url @@ -173,17 +148,11 @@ def __post_init__(self): ) # validations - # defined fields without a default must have a value + # defined fields must have a not None value for field in dataclasses.fields(self): value = getattr(self, field.name) - if field.default == dataclasses.MISSING and value in ( - dataclasses.MISSING, - None, - ): + if value in (dataclasses.MISSING, None): raise ValueError(f"{field.name} must be set") - # storage_password must be set - if self.storage_password is None: - raise ValueError("storage_password must be set") @property def storage_kws(self) -> dict[str, str | bool | None]: diff --git a/cads_catalogue/database.py b/cads_catalogue/database.py index 03e771d..7cfc81f 100644 --- a/cads_catalogue/database.py +++ b/cads_catalogue/database.py @@ -294,7 +294,7 @@ def ensure_session_obj(read_only: bool = False) -> sa.orm.sessionmaker: settings = config.ensure_settings(config.dbsettings) if read_only: engine = sa.create_engine( - settings.connection_string_ro, pool_recycle=settings.pool_recycle + settings.connection_string_read, pool_recycle=settings.pool_recycle ) else: engine = sa.create_engine( diff --git a/tests/test_01_config.py b/tests/test_01_config.py index 8123021..8e3e701 100644 --- a/tests/test_01_config.py +++ b/tests/test_01_config.py @@ -8,60 +8,45 @@ def test_sqlalchemysettings(temp_environ: Any) -> None: # check settings must have a password set (no default) + temp_environ.update( + dict( + catalogue_db_host="host1", + catalogue_db_host_read="host2", + catalogue_db_name="dbname", + catalogue_db_user="dbuser", + ) + ) temp_environ.pop("catalogue_db_password", default=None) - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError): config.SqlalchemySettings() - assert "read_db_user" in str(excinfo.value) # also an empty password can be set - temp_environ["catalogue_db_user"] = "user1" - temp_environ["catalogue_db_host"] = "host1" - temp_environ["catalogue_db_name"] = "dbname1" - settings = config.SqlalchemySettings(catalogue_db_password="") + settings = config.SqlalchemySettings( + catalogue_db_password="", + catalogue_db_host="host1", + catalogue_db_host_read="host2", + catalogue_db_name="dbname1", + catalogue_db_user="user1", + ) assert settings.catalogue_db_password == "" + config.dbsettings = None - # check backward compatibility defaults - assert settings.__dict__ == { - "catalogue_db_user": "user1", - "catalogue_db_password": "", - "catalogue_db_host": "host1", - "catalogue_db_name": "dbname1", - "read_db_user": "user1", - "read_db_password": "", - "write_db_user": "user1", - "write_db_password": "", - "db_host": "host1", - "pool_recycle": 60, - "match_args": {"catalogue_db_password": ""}, - } - - # use not backward compatibility variables - temp_environ["read_db_user"] = "ro_user" - temp_environ["read_db_password"] = "ro_password" - temp_environ["write_db_user"] = "rw_user" - temp_environ["write_db_password"] = "rw_password" - temp_environ["db_host"] = "new_db_host" - settings = config.SqlalchemySettings() - assert settings.__dict__ == { - "catalogue_db_user": "user1", - "catalogue_db_password": None, - "catalogue_db_host": "host1", - "catalogue_db_name": "dbname1", - "read_db_user": "ro_user", - "read_db_password": "ro_password", - "write_db_user": "rw_user", - "write_db_password": "rw_password", - "db_host": "new_db_host", - "pool_recycle": 60, - "match_args": {}, - } - assert ( - settings.connection_string - == "postgresql://rw_user:rw_password@new_db_host/dbname1" + # also a not empty password can be set + temp_environ.update( + dict( + catalogue_db_password="apassword", + catalogue_db_host="host1", + catalogue_db_host_read="host2", + catalogue_db_name="dbname", + catalogue_db_user="dbuser", + ) ) + settings = config.SqlalchemySettings() + assert settings.catalogue_db_password == "apassword" + config.dbsettings = None + assert settings.connection_string == "postgresql://dbuser:apassword@host1/dbname" assert ( - settings.connection_string_ro - == "postgresql://ro_user:ro_password@new_db_host/dbname1" + settings.connection_string_read == "postgresql://dbuser:apassword@host2/dbname" ) @@ -69,6 +54,7 @@ def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> temp_environ["catalogue_db_user"] = "auser" temp_environ["catalogue_db_password"] = "apassword" temp_environ["catalogue_db_host"] = "ahost" + temp_environ["catalogue_db_host_read"] = "ahost2" temp_environ["catalogue_db_name"] = "aname" # initially global settings is importable, but it is None assert config.dbsettings is None @@ -80,8 +66,8 @@ def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> == "postgresql://auser:apassword@ahost/aname" ) assert ( - effective_settings.connection_string_ro - == "postgresql://auser:apassword@ahost/aname" + effective_settings.connection_string_read + == "postgresql://auser:apassword@ahost2/aname" ) assert config.dbsettings == effective_settings config.dbsettings = None @@ -91,20 +77,16 @@ def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> "catalogue_db_user": "monica", "catalogue_db_password": "secret1", "catalogue_db_host": "myhost", + "catalogue_db_host_read": "myhost2", "catalogue_db_name": "mybroker", - "read_db_user": "ro_user", - "read_db_password": "ro_password", - "write_db_user": "rw_user", - "write_db_password": "rw_password", - "db_host": "new_db_host", } my_settings_connection_string = ( - "postgresql://%(write_db_user)s:%(write_db_password)s" - "@%(db_host)s/%(catalogue_db_name)s" % my_settings_dict + "postgresql://%(catalogue_db_user)s:%(catalogue_db_password)s" + "@%(catalogue_db_host)s/%(catalogue_db_name)s" % my_settings_dict ) my_settings_connection_string_ro = ( - "postgresql://%(read_db_user)s:%(read_db_password)s" - "@%(db_host)s/%(catalogue_db_name)s" % my_settings_dict + "postgresql://%(catalogue_db_user)s:%(catalogue_db_password)s" + "@%(catalogue_db_host_read)s/%(catalogue_db_name)s" % my_settings_dict ) mysettings = config.SqlalchemySettings(**my_settings_dict) effective_settings = config.ensure_settings(mysettings) @@ -112,7 +94,7 @@ def test_ensure_settings(session_obj: sa.orm.sessionmaker, temp_environ: Any) -> assert config.dbsettings == effective_settings assert effective_settings == mysettings assert effective_settings.connection_string == my_settings_connection_string - assert effective_settings.connection_string_ro == my_settings_connection_string_ro + assert effective_settings.connection_string_read == my_settings_connection_string_ro config.dbsettings = None diff --git a/tests/test_02_database.py b/tests/test_02_database.py index 999e00f..9fb6a20 100644 --- a/tests/test_02_database.py +++ b/tests/test_02_database.py @@ -29,14 +29,13 @@ def test_init_database(postgresql: Connection[str]) -> None: def test_ensure_session_obj(postgresql: Connection[str], temp_environ: Any) -> None: temp_environ["catalogue_db_host"] = "cataloguehost" temp_environ["catalogue_db_password"] = postgresql.info.password - temp_environ["read_db_user"] = "readonlyuser" - temp_environ["write_db_user"] = "writeuser" + temp_environ["catalogue_db_user"] = "user" + temp_environ["catalogue_db_host_read"] = "read_only_host" temp_environ["catalogue_db_name"] = "catalogue" ret_value = database.ensure_session_obj() assert isinstance(ret_value, sessionmaker) assert ( - str(ret_value.kw["bind"].url) - == "postgresql://writeuser:***@cataloguehost/catalogue" + str(ret_value.kw["bind"].url) == "postgresql://user:***@cataloguehost/catalogue" ) config.dbsettings = None @@ -44,6 +43,6 @@ def test_ensure_session_obj(postgresql: Connection[str], temp_environ: Any) -> N assert isinstance(ret_value, sessionmaker) assert ( str(ret_value.kw["bind"].url) - == "postgresql://readonlyuser:***@cataloguehost/catalogue" + == "postgresql://user:***@read_only_host/catalogue" ) config.dbsettings = None