Skip to content

Commit 78a94fb

Browse files
kushalbakshiclaude
andcommitted
Address PR review: rename database.dbname → database.name, deprecate database_prefix
Review feedback from PR datajoint#1426: 1. Rename setting to database.name (env: DJ_DATABASE_NAME) to match section naming style and avoid stutter. Connection kwarg is database_name. Adapter still receives dbname (psycopg2's parameter). 2. Deprecate database_prefix — emit DeprecationWarning when non-empty. Will be removed in DataJoint 2.3. database.name is the replacement. 3. Revert version.py to 2.2.0 — release workflow owns version bumps. 4. Warn when database.name is set with MySQL backend (MySQL does not support database selection via this parameter). 5. Include database name in Connection.__repr__ and log message when set. Format: user@host:port/database_name Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 35e93b0 commit 78a94fb

File tree

4 files changed

+101
-41
lines changed

4 files changed

+101
-41
lines changed

src/datajoint/connection.py

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ def __init__(
168168
port: int | None = None,
169169
use_tls: bool | dict | None = None,
170170
*,
171-
dbname: str | None = None,
171+
database_name: str | None = None,
172172
backend: str | None = None,
173173
config_override: "Config | None" = None,
174174
) -> None:
@@ -181,9 +181,9 @@ def __init__(
181181
port = int(port)
182182
elif port is None:
183183
port = self._config["database.port"]
184-
if dbname is None:
185-
dbname = self._config.get("database.dbname")
186-
self.conn_info = dict(host=host, port=port, user=user, passwd=password, dbname=dbname)
184+
if database_name is None:
185+
database_name = self._config.get("database.name")
186+
self.conn_info = dict(host=host, port=port, user=user, passwd=password, database_name=database_name)
187187
if use_tls is not False:
188188
# use_tls can be: None (auto-detect), True (enable), False (disable), or dict (custom config)
189189
if isinstance(use_tls, dict):
@@ -204,12 +204,27 @@ def __init__(
204204
backend = self._config["database.backend"]
205205
self.adapter = get_adapter(backend)
206206

207+
if database_name and self.adapter.backend == "mysql":
208+
warnings.warn(
209+
"database.name is set but the MySQL backend does not support database selection. "
210+
"This setting only applies to PostgreSQL connections.",
211+
UserWarning,
212+
stacklevel=2,
213+
)
214+
207215
self.connect()
208216
if self.is_connected:
209-
logger.info("DataJoint {version} connected to {user}@{host}:{port}".format(version=__version__, **self.conn_info))
217+
db = self.conn_info.get("database_name")
218+
db_str = f"/{db}" if db else ""
219+
logger.info(
220+
f"DataJoint {__version__} connected to "
221+
f"{self.conn_info['user']}@{self.conn_info['host']}:{self.conn_info['port']}{db_str}"
222+
)
210223
self.connection_id = self.adapter.get_connection_id(self._conn)
211224
else:
212-
raise errors.LostConnectionError("Connection failed {user}@{host}:{port}".format(**self.conn_info))
225+
raise errors.LostConnectionError(
226+
f"Connection failed {self.conn_info['user']}@{self.conn_info['host']}:{self.conn_info['port']}"
227+
)
213228
self._in_transaction = False
214229
self.schemas = dict()
215230
self.dependencies = Dependencies(self)
@@ -219,7 +234,9 @@ def __eq__(self, other):
219234

220235
def __repr__(self):
221236
connected = "connected" if self.is_connected else "disconnected"
222-
return "DataJoint connection ({connected}) {user}@{host}:{port}".format(connected=connected, **self.conn_info)
237+
db = self.conn_info.get("database_name")
238+
db_str = f"/{db}" if db else ""
239+
return f"DataJoint connection ({connected}) {self.conn_info['user']}@{self.conn_info['host']}:{self.conn_info['port']}{db_str}"
223240

224241
def _build_connect_kwargs(self, use_tls=None):
225242
"""Build kwargs dict for adapter.connect()."""
@@ -231,8 +248,8 @@ def _build_connect_kwargs(self, use_tls=None):
231248
charset=self._config["connection.charset"],
232249
use_tls=use_tls if use_tls is not None else self.conn_info.get("ssl"),
233250
)
234-
if self.conn_info.get("dbname"):
235-
kwargs["dbname"] = self.conn_info["dbname"]
251+
if self.conn_info.get("database_name"):
252+
kwargs["dbname"] = self.conn_info["database_name"]
236253
return kwargs
237254

238255
def connect(self) -> None:

src/datajoint/settings.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
"database.password": "DJ_PASS",
6666
"database.backend": "DJ_BACKEND",
6767
"database.port": "DJ_PORT",
68+
"database.name": "DJ_DATABASE_NAME",
6869
"database.database_prefix": "DJ_DATABASE_PREFIX",
6970
"database.create_tables": "DJ_CREATE_TABLES",
7071
"loglevel": "DJ_LOG_LEVEL",
@@ -196,18 +197,17 @@ class DatabaseSettings(BaseSettings):
196197
description="Database backend: 'mysql' or 'postgresql'",
197198
)
198199
port: int | None = Field(default=None, validation_alias="DJ_PORT")
199-
dbname: str | None = Field(
200+
name: str | None = Field(
200201
default=None,
201-
validation_alias="DJ_DBNAME",
202+
validation_alias="DJ_DATABASE_NAME",
202203
description="Database name for PostgreSQL connections. Defaults to 'postgres' if not set.",
203204
)
204205
reconnect: bool = True
205206
use_tls: bool | None = Field(default=None, validation_alias="DJ_USE_TLS")
206207
database_prefix: str = Field(
207208
default="",
208209
validation_alias="DJ_DATABASE_PREFIX",
209-
description="Prefix for database/schema names. "
210-
"Not automatically applied; use dj.config.database.database_prefix when creating schemas.",
210+
description="Deprecated. Use database.name instead.",
211211
)
212212
create_tables: bool = Field(
213213
default=True,
@@ -223,6 +223,20 @@ def set_default_port_from_backend(self) -> "DatabaseSettings":
223223
self.port = 5432 if self.backend == "postgresql" else 3306
224224
return self
225225

226+
@model_validator(mode="after")
227+
def warn_database_prefix_deprecated(self) -> "DatabaseSettings":
228+
"""Emit deprecation warning when database_prefix is set."""
229+
if self.database_prefix:
230+
import warnings
231+
232+
warnings.warn(
233+
"database_prefix is deprecated and will be removed in DataJoint 2.3. "
234+
"Use database.name to select a PostgreSQL database instead.",
235+
DeprecationWarning,
236+
stacklevel=2,
237+
)
238+
return self
239+
226240

227241
class ConnectionSettings(BaseSettings):
228242
"""Connection behavior settings."""

src/datajoint/version.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
# version bump auto managed by Github Actions:
22
# label_prs.yaml(prep), release.yaml(bump), post_release.yaml(edit)
33
# manually set this version will be eventually overwritten by the above actions
4-
__version__ = "2.2.1"
4+
__version__ = "2.2.0"

tests/unit/test_settings.py

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -750,55 +750,84 @@ def test_similar_prefix_names_allowed(self):
750750
dj.config.stores.update(original_stores)
751751

752752

753-
class TestDbnameConfiguration:
754-
"""Test database.dbname configuration."""
753+
class TestDatabaseNameConfiguration:
754+
"""Test database.name configuration."""
755755

756-
def test_dbname_default_is_none(self):
757-
"""Dbname defaults to None when not configured."""
756+
def test_database_name_default_is_none(self):
757+
"""Database name defaults to None when not configured."""
758758
from datajoint.settings import DatabaseSettings
759759

760760
s = DatabaseSettings()
761-
assert s.dbname is None
761+
assert s.name is None
762762

763-
def test_dbname_env_var(self, monkeypatch):
764-
"""DJ_DBNAME environment variable sets dbname."""
763+
def test_database_name_env_var(self, monkeypatch):
764+
"""DJ_DATABASE_NAME environment variable sets database name."""
765765
from datajoint.settings import DatabaseSettings
766766

767-
monkeypatch.setenv("DJ_DBNAME", "my_database")
767+
monkeypatch.setenv("DJ_DATABASE_NAME", "my_database")
768768
s = DatabaseSettings()
769-
assert s.dbname == "my_database"
769+
assert s.name == "my_database"
770770

771-
def test_dbname_from_config_file(self, tmp_path, monkeypatch):
772-
"""Load dbname from config file."""
771+
def test_database_name_from_config_file(self, tmp_path, monkeypatch):
772+
"""Load database name from config file."""
773773
import json
774774

775775
from datajoint.settings import Config
776776

777777
config_file = tmp_path / "test_config.json"
778-
config_file.write_text(json.dumps({"database": {"dbname": "custom_db", "host": "localhost"}}))
778+
config_file.write_text(json.dumps({"database": {"name": "custom_db", "host": "localhost"}}))
779779

780-
monkeypatch.delenv("DJ_DBNAME", raising=False)
780+
monkeypatch.delenv("DJ_DATABASE_NAME", raising=False)
781781
monkeypatch.delenv("DJ_HOST", raising=False)
782782

783783
cfg = Config()
784784
cfg.load(config_file)
785-
assert cfg.database.dbname == "custom_db"
785+
assert cfg.database.name == "custom_db"
786786

787-
def test_dbname_dict_access(self):
788-
"""Dict-style access reads and writes dbname."""
789-
original = dj.config.database.dbname
787+
def test_database_name_dict_access(self):
788+
"""Dict-style access reads and writes database name."""
789+
original = dj.config.database.name
790790
try:
791-
dj.config.database.dbname = "test_db"
792-
assert dj.config["database.dbname"] == "test_db"
791+
dj.config.database.name = "test_db"
792+
assert dj.config["database.name"] == "test_db"
793793
finally:
794-
dj.config.database.dbname = original
795-
796-
def test_dbname_override_context_manager(self):
797-
"""Override context manager temporarily sets dbname."""
798-
original = dj.config.database.dbname
799-
with dj.config.override(database__dbname="override_db"):
800-
assert dj.config.database.dbname == "override_db"
801-
assert dj.config.database.dbname == original
794+
dj.config.database.name = original
795+
796+
def test_database_name_override_context_manager(self):
797+
"""Override context manager temporarily sets database name."""
798+
original = dj.config.database.name
799+
with dj.config.override(database__name="override_db"):
800+
assert dj.config.database.name == "override_db"
801+
assert dj.config.database.name == original
802+
803+
def test_database_prefix_deprecation_warning(self, monkeypatch):
804+
"""Non-empty database_prefix emits DeprecationWarning."""
805+
import warnings
806+
807+
from datajoint.settings import DatabaseSettings
808+
809+
monkeypatch.setenv("DJ_DATABASE_PREFIX", "test_")
810+
with warnings.catch_warnings(record=True) as w:
811+
warnings.simplefilter("always")
812+
DatabaseSettings()
813+
deprecation_warnings = [
814+
x for x in w if issubclass(x.category, DeprecationWarning) and "database_prefix" in str(x.message)
815+
]
816+
assert len(deprecation_warnings) >= 1
817+
818+
def test_database_prefix_empty_no_warning(self):
819+
"""Empty database_prefix does not emit DeprecationWarning."""
820+
import warnings
821+
822+
from datajoint.settings import DatabaseSettings
823+
824+
with warnings.catch_warnings(record=True) as w:
825+
warnings.simplefilter("always")
826+
DatabaseSettings()
827+
deprecation_warnings = [
828+
x for x in w if issubclass(x.category, DeprecationWarning) and "database_prefix" in str(x.message)
829+
]
830+
assert len(deprecation_warnings) == 0
802831

803832

804833
class TestBackendConfiguration:

0 commit comments

Comments
 (0)