diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 64ad3f6ab..25d98470e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,17 +36,17 @@ pytest tests/ ## Running Tests -Tests use [testcontainers](https://testcontainers.com/) to automatically manage MySQL, PostgreSQL, and MinIO containers. No manual `docker-compose up` required. +Tests use [testcontainers](https://testcontainers.com/) to automatically manage PostgreSQL, MySQL, and MinIO containers. No manual `docker-compose up` required. -Integration tests are **backend-parameterized** — tests using the `backend` fixture run automatically against both MySQL and PostgreSQL. +Integration tests are **backend-parameterized** — tests using the `backend` fixture run automatically against both PostgreSQL and MySQL. ```bash pixi run -e test test # All tests (both backends) pixi run -e test test-cov # With coverage pixi run -e test pytest tests/unit/ # Unit tests only pixi run -e test pytest tests/integration/test_blob.py -v # Specific file -pixi run -e test pytest -m mysql # MySQL tests only pixi run -e test pytest -m postgresql # PostgreSQL tests only +pixi run -e test pytest -m mysql # MySQL tests only ``` **macOS Docker Desktop users:** If tests fail to connect: @@ -62,18 +62,18 @@ DataJoint supports MySQL 8.0.13+ and PostgreSQL 15+ as production database backe pip install -e ".[postgres]" # Installs psycopg2-binary ``` -Tests automatically spin up both MySQL and PostgreSQL containers via testcontainers. Backend-parameterized tests (those using the `backend` fixture in `tests/conftest.py`) run against both backends to ensure feature parity. +Tests automatically spin up both PostgreSQL and MySQL containers via testcontainers. Backend-parameterized tests (those using the `backend` fixture in `tests/conftest.py`) run against both backends to ensure feature parity. PostgreSQL is the default backend (`database.backend`), so fixtures and tests that need MySQL select it explicitly. ### External Containers (for debugging) ```bash -# MySQL + MinIO -docker compose up -d db minio +# PostgreSQL + MySQL + MinIO +docker compose up -d postgres db minio DJ_USE_EXTERNAL_CONTAINERS=1 pixi run -e test test docker compose down -# MySQL + PostgreSQL + MinIO -docker compose up -d db postgres minio +# MySQL + MinIO +docker compose up -d db minio DJ_USE_EXTERNAL_CONTAINERS=1 pixi run -e test test docker compose down ``` @@ -111,6 +111,15 @@ Hooks include: **ruff** (lint/format), **codespell**, YAML/JSON/TOML validation. For `DJ_USE_EXTERNAL_CONTAINERS=1`: +### PostgreSQL + +| Variable | Default | Description | +|----------|---------|-------------| +| `DJ_PG_HOST` | `localhost` | PostgreSQL hostname | +| `DJ_PG_PORT` | `5432` | PostgreSQL port | +| `DJ_PG_USER` | `postgres` | PostgreSQL username | +| `DJ_PG_PASS` | `password` | PostgreSQL password | + ### MySQL | Variable | Default | Description | @@ -120,14 +129,7 @@ For `DJ_USE_EXTERNAL_CONTAINERS=1`: | `DJ_USER` | `root` | MySQL username | | `DJ_PASS` | `password` | MySQL password | -### PostgreSQL - -| Variable | Default | Description | -|----------|---------|-------------| -| `DJ_PG_HOST` | `localhost` | PostgreSQL hostname | -| `DJ_PG_PORT` | `5432` | PostgreSQL port | -| `DJ_PG_USER` | `postgres` | PostgreSQL username | -| `DJ_PG_PASS` | `password` | PostgreSQL password | +The test suite reads `DJ_HOST`, `DJ_PORT`, `DJ_USER`, and `DJ_PASS` as the MySQL server. Outside the tests, these variables configure whichever backend `DJ_BACKEND` selects, PostgreSQL by default. ### Object Storage diff --git a/docker-compose.yaml b/docker-compose.yaml index 23fd773c1..8de571804 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,4 +1,4 @@ -# Development environment with MySQL and MinIO services +# Development environment with PostgreSQL, MySQL, and MinIO services # # NOTE: docker-compose is OPTIONAL for running tests. # Tests use testcontainers to automatically manage containers. @@ -6,24 +6,14 @@ # # Use docker-compose for development/debugging when you want # persistent containers that survive test runs: -# docker compose up -d db minio # Start services manually -# pytest tests/ # Tests will use these containers +# docker compose up -d postgres db minio # Start services manually +# DJ_USE_EXTERNAL_CONTAINERS=1 pytest tests/ # Tests will use these containers +# +# PostgreSQL is DataJoint's default backend. The MySQL service is `db`. # # Full Docker testing (CI): # docker compose --profile test up djtest --build services: - db: - image: datajoint/mysql:${MYSQL_VER:-8.0} - environment: - - MYSQL_ROOT_PASSWORD=${DJ_PASS:-password} - command: mysqld --default-authentication-plugin=mysql_native_password - ports: - - "3306:3306" - healthcheck: - test: [ "CMD", "mysqladmin", "ping", "-h", "localhost" ] - timeout: 30s - retries: 5 - interval: 15s postgres: image: postgres:${POSTGRES_VER:-15} environment: @@ -37,6 +27,18 @@ services: timeout: 30s retries: 5 interval: 15s + db: + image: datajoint/mysql:${MYSQL_VER:-8.0} + environment: + - MYSQL_ROOT_PASSWORD=${DJ_PASS:-password} + command: mysqld --default-authentication-plugin=mysql_native_password + ports: + - "3306:3306" + healthcheck: + test: [ "CMD", "mysqladmin", "ping", "-h", "localhost" ] + timeout: 30s + retries: 5 + interval: 15s minio: image: minio/minio:${MINIO_VER:-RELEASE.2025-02-28T09-55-16Z} environment: @@ -63,23 +65,26 @@ services: PY_VER: ${PY_VER:-3.10} HOST_UID: ${HOST_UID:-1000} depends_on: - db: - condition: service_healthy postgres: condition: service_healthy + db: + condition: service_healthy minio: condition: service_healthy environment: + - DJ_PG_HOST=postgres + - DJ_PG_USER=postgres + - DJ_PG_PASS=password + - DJ_PG_PORT=5432 + # DJ_HOST, DJ_USER, and DJ_PASS name the MySQL service, which the test suite + # reads as its MySQL server, so DJ_BACKEND selects the matching backend. + - DJ_BACKEND=mysql - DJ_HOST=db - DJ_USER=root - DJ_PASS=password - DJ_TEST_HOST=db - DJ_TEST_USER=datajoint - DJ_TEST_PASSWORD=datajoint - - DJ_PG_HOST=postgres - - DJ_PG_USER=postgres - - DJ_PG_PASS=password - - DJ_PG_PORT=5432 - S3_ENDPOINT=minio:9000 - S3_ACCESS_KEY=datajoint - S3_SECRET_KEY=datajoint diff --git a/src/datajoint/cli.py b/src/datajoint/cli.py index c77cca686..1310980b4 100644 --- a/src/datajoint/cli.py +++ b/src/datajoint/cli.py @@ -5,8 +5,8 @@ Usage:: - # Start REPL with database credentials - dj --user root --password secret --host localhost:3306 + # Start REPL with database credentials (PostgreSQL unless DJ_BACKEND=mysql) + dj --user postgres --password secret --host localhost:5432 # Load schemas as virtual modules dj -s my_lab:lab -s my_analysis:analysis @@ -41,7 +41,8 @@ def cli(args: list[str] | None = None) -> None: -------- From the command line:: - $ dj --host localhost:3306 --user root --password secret + $ dj --host localhost:5432 --user postgres --password secret + $ DJ_BACKEND=mysql dj --host localhost:3306 --user root --password secret $ dj -s my_lab:lab -s my_analysis:analysis Programmatically:: @@ -52,7 +53,11 @@ def cli(args: list[str] | None = None) -> None: parser = argparse.ArgumentParser( prog="dj", description="DataJoint interactive console. Start a Python REPL with DataJoint pre-loaded.", - epilog="Example: dj -s my_lab:lab --host localhost:3306", + epilog=( + "The database backend is PostgreSQL unless database.backend is set in config; " + "for a MySQL server, set DJ_BACKEND=mysql. " + "Example: dj -s my_lab:lab --host localhost:5432" + ), ) parser.add_argument( "-V", diff --git a/src/datajoint/connection.py b/src/datajoint/connection.py index 4e6252e0d..4f34b53e1 100644 --- a/src/datajoint/connection.py +++ b/src/datajoint/connection.py @@ -29,6 +29,13 @@ cache_key = "query_cache" # the key to lookup the query_cache folder in dj.config +# Added to a failed connection whose backend came from the default rather than from the +# configuration, since a MySQL server reached with the default backend fails this way. +DEFAULT_BACKEND_HINT = ( + "The default backend is postgresql since DataJoint 2.4; " + "set database.backend='mysql' (DJ_BACKEND=mysql) for a MySQL server." +) + def translate_query_error(client_error: Exception, query: str, adapter) -> Exception: """ @@ -163,6 +170,16 @@ class Connection: Port number. Overridden if specified in host. use_tls : bool or dict, optional TLS encryption option. + backend : str, optional + Database backend: ``"postgresql"`` or ``"mysql"``. Default from + ``database.backend`` in config, which is ``"postgresql"`` unless set. + + Raises + ------ + DataJointError + If the connection fails while the backend is the default rather than one + set by the ``backend`` argument or in config. The message adds how to + select the MySQL backend. Attributes ---------- @@ -211,20 +228,27 @@ def __init__( self._query_cache = None self._is_closed = True # Mark as closed until connect() succeeds - # Select adapter: explicit backend > config backend + # Select adapter: explicit backend > config backend. When neither names one, + # the backend is the default and a failure to connect says how to change it. + backend_is_default = backend is None and "backend" not in self._config.database.model_fields_set if backend is None: backend = self._config["database.backend"] - self.adapter = get_adapter(backend) - - if database_name and self.adapter.backend == "mysql": - warnings.warn( - "database.name is set but the MySQL backend does not support database selection. " - "This setting only applies to PostgreSQL connections.", - UserWarning, - stacklevel=2, - ) + try: + self.adapter = get_adapter(backend) + + if database_name and self.adapter.backend == "mysql": + warnings.warn( + "database.name is set but the MySQL backend does not support database selection. " + "This setting only applies to PostgreSQL connections.", + UserWarning, + stacklevel=2, + ) - self.connect() + self.connect() + except Exception as error: + if backend_is_default: + raise errors.DataJointError(f"{str(error).rstrip()}\n{DEFAULT_BACKEND_HINT}") from error + raise if self.is_connected: db = self.conn_info.get("database_name") db_str = f"/{db}" if db else "" diff --git a/src/datajoint/instance.py b/src/datajoint/instance.py index 455336a7c..80bcc5fe9 100644 --- a/src/datajoint/instance.py +++ b/src/datajoint/instance.py @@ -54,11 +54,12 @@ class Instance: password : str Database password. port : int, optional - Database port. Defaults to 3306 for MySQL, 5432 for PostgreSQL. + Database port. Defaults to 5432 for PostgreSQL, 3306 for MySQL. use_tls : bool or dict, optional TLS configuration. backend : str, optional - Database backend: ``"mysql"`` or ``"postgresql"``. Default from config. + Database backend: ``"postgresql"`` or ``"mysql"``. Default from config, + which is ``"postgresql"`` unless set. **kwargs : Any Additional config overrides applied to this instance's config. @@ -112,14 +113,14 @@ def __init__( if port is None: port = self.config.database.port - # Create connection with this instance's config and backend + # Create connection with this instance's config. The backend comes from that + # config, which records whether it was set or left at the default. self.connection = Connection( host, user, password, port, use_tls, - backend=self.config.database.backend, config_override=self.config, ) diff --git a/src/datajoint/settings.py b/src/datajoint/settings.py index 38339a1d9..008f5babe 100644 --- a/src/datajoint/settings.py +++ b/src/datajoint/settings.py @@ -21,10 +21,10 @@ >>> import datajoint as dj >>> dj.config.database.host 'localhost' ->>> dj.config.database.backend -'mysql' ->>> dj.config.database.port # Auto-detects: 3306 for MySQL, 5432 for PostgreSQL -3306 +>>> dj.config.database.backend # PostgreSQL is the default; set 'mysql' for a MySQL server +'postgresql' +>>> dj.config.database.port # Follows the backend unless set: 5432 for PostgreSQL, 3306 for MySQL +5432 >>> with dj.config.override(safemode=False): ... # dangerous operations here ... pass @@ -199,11 +199,15 @@ class DatabaseSettings(BaseSettings): user: str | None = Field(default=None, validation_alias="DJ_USER") password: SecretStr | None = Field(default=None, validation_alias="DJ_PASS") backend: Literal["mysql", "postgresql"] = Field( - default="mysql", + default="postgresql", validation_alias="DJ_BACKEND", - description="Database backend: 'mysql' or 'postgresql'", + description="Database backend: 'postgresql' (default) or 'mysql'", + ) + port: int | None = Field( + default=None, + validation_alias="DJ_PORT", + description="Database port. Defaults to the backend's port: 5432 for PostgreSQL, 3306 for MySQL.", ) - port: int | None = Field(default=None, validation_alias="DJ_PORT") name: str | None = Field( default=None, validation_alias="DJ_DATABASE_NAME", @@ -225,9 +229,19 @@ class DatabaseSettings(BaseSettings): @model_validator(mode="after") def set_default_port_from_backend(self) -> "DatabaseSettings": - """Set default port based on backend if not explicitly provided.""" + """ + Use the backend's default port unless a port was set explicitly. + + The validator also runs when a setting is assigned, so a port that was + not set follows later changes of the backend (for example + ``backend: "mysql"`` in ``datajoint.json``). Setting the port to None + returns it to the backend's default. + """ if self.port is None: - self.port = 5432 if self.backend == "postgresql" else 3306 + self.model_fields_set.discard("port") + if "port" not in self.model_fields_set: + # Write the derived port directly so that it is not recorded as set explicitly. + self.__dict__["port"] = 5432 if self.backend == "postgresql" else 3306 return self @@ -939,6 +953,9 @@ def save_template( Set ``DJ_IGNORE_CONFIG_FILE=true`` to skip both ``datajoint.json`` and the secrets directory entirely (env-var-only configuration). + The template selects the PostgreSQL backend on port 5432. For a MySQL + server, set ``backend`` to ``"mysql"`` and ``port`` to 3306. + Parameters ---------- path : str or Path, optional @@ -973,15 +990,17 @@ def save_template( if minimal: template = { "database": { + "backend": "postgresql", "host": "localhost", - "port": 3306, + "port": 5432, }, } else: template = { "database": { + "backend": "postgresql", "host": "localhost", - "port": 3306, + "port": 5432, "reconnect": True, "use_tls": None, }, diff --git a/tests/conftest.py b/tests/conftest.py index bfbbc293d..35d3c36a0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,10 @@ DJ_USE_EXTERNAL_CONTAINERS=1 DJ_HOST=localhost DJ_PORT=3306 S3_ENDPOINT=localhost:9000 pytest +PostgreSQL is the default backend. The MySQL-only fixtures (``db_creds_root``, +``db_creds_test`` and the connections built from them) select MySQL explicitly +through ``mysql_backend_config``. + To run only unit tests (no Docker required): pytest -m "not requires_mysql" """ @@ -207,7 +211,19 @@ def prefix(): @pytest.fixture(scope="session") -def db_creds_root(mysql_container) -> Dict: +def mysql_backend_config(): + """Select the MySQL backend in ``dj.config`` for the MySQL-only fixtures and tests. + + PostgreSQL is the default backend. Tests connect to the MySQL server with + ``dj.conn(**db_creds_root)`` and ``dj.Connection(**db_creds_test)``, which take + the backend from ``dj.config``, so the MySQL credentials fixtures set it. + ``connection_by_backend`` switches it per test and restores it afterwards. + """ + dj.config["database.backend"] = "mysql" + + +@pytest.fixture(scope="session") +def db_creds_root(mysql_container, mysql_backend_config) -> Dict: """Root database credentials from container or environment.""" if mysql_container is not None: # From testcontainer @@ -230,7 +246,7 @@ def db_creds_root(mysql_container) -> Dict: @pytest.fixture(scope="session") -def db_creds_test(mysql_container) -> Dict: +def db_creds_test(mysql_container, mysql_backend_config) -> Dict: """Test user database credentials from container or environment.""" if mysql_container is not None: # From testcontainer @@ -340,7 +356,7 @@ def connection_by_backend(db_creds_by_backend): is restored after each test, preventing config pollution between tests. """ # Save original config to restore after tests - original_backend = dj.config.get("database.backend", "mysql") + original_backend = dj.config.get("database.backend", "postgresql") original_host = dj.config.get("database.host") original_port = dj.config.get("database.port") @@ -410,7 +426,7 @@ def configure_datajoint(db_creds_root): @pytest.fixture(scope="session") def connection_root_bare(db_creds_root, configure_datajoint): """Bare root connection without user setup.""" - connection = dj.Connection(**db_creds_root) + connection = dj.Connection(**db_creds_root, backend="mysql") yield connection @@ -480,7 +496,7 @@ def connection_test(connection_root, prefix, db_creds_test): """ ) - connection = dj.Connection(**db_creds_test) + connection = dj.Connection(**db_creds_test, backend="mysql") yield connection connection_root.query(f"""DROP USER `{db_creds_test["user"]}`""") connection.close() diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index 1f8144f0f..524c18c2f 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -2,6 +2,7 @@ Collection of test cases to test the dj cli """ +import os import subprocess import sys @@ -80,7 +81,8 @@ class IJ(dj.Lookup): """ contents = list(dict(i=i, j=j + 2) for i in range(3) for j in range(3)) - # Pass credentials via CLI args to avoid prompting for username + # Pass credentials via CLI args to avoid prompting for username. The test server + # is MySQL and PostgreSQL is the default backend, so select MySQL for the subprocess. process = subprocess.Popen( [ sys.executable, @@ -99,6 +101,7 @@ class IJ(dj.Lookup): stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + env={**os.environ, "DJ_BACKEND": "mysql"}, ) process.stdin.write("test_schema.__dict__['__name__']\n") diff --git a/tests/unit/test_connection_backend_hint.py b/tests/unit/test_connection_backend_hint.py new file mode 100644 index 000000000..80b9b93b8 --- /dev/null +++ b/tests/unit/test_connection_backend_hint.py @@ -0,0 +1,102 @@ +""" +Unit tests for the hint added when a connection with the default backend fails. + +PostgreSQL is the default backend. A connection that fails while the backend +was left at that default says how to select MySQL; one whose backend was set, +by argument, config assignment, ``datajoint.json`` or ``DJ_BACKEND``, raises +the original error unchanged. No database server is needed. +""" + +import json +import socket +from unittest.mock import patch + +import pytest + +from datajoint.connection import DEFAULT_BACKEND_HINT, Connection +from datajoint.errors import DataJointError +from datajoint.settings import Config + + +class _RefusingAdapter: + """Adapter stub whose connect fails the way a refused connection does.""" + + backend = "postgresql" + + def connect(self, **kwargs): + raise OSError("connection refused") + + +@pytest.fixture +def fresh_config(monkeypatch): + """A Config built without DJ_BACKEND or DJ_PORT from the developer's shell.""" + for var in ("DJ_BACKEND", "DJ_PORT"): + monkeypatch.delenv(var, raising=False) + return Config() + + +def _connect(config, **kwargs): + with patch("datajoint.connection.get_adapter", return_value=_RefusingAdapter()): + return Connection("localhost", "user", "password", config_override=config, **kwargs) + + +def test_default_backend_failure_adds_hint(fresh_config): + """A failed connection with the default backend names the MySQL setting.""" + with pytest.raises(DataJointError) as info: + _connect(fresh_config) + message = str(info.value) + assert "connection refused" in message + assert DEFAULT_BACKEND_HINT in message + assert "database.backend='mysql'" in message and "DJ_BACKEND=mysql" in message + assert isinstance(info.value.__cause__, OSError) + + +def test_missing_driver_with_default_backend_adds_hint(fresh_config): + """A missing PostgreSQL driver with the default backend also names the MySQL setting.""" + missing = ImportError("psycopg2 is required for PostgreSQL support.") + with patch("datajoint.connection.get_adapter", side_effect=missing): + with pytest.raises(DataJointError, match="psycopg2 is required") as info: + Connection("localhost", "user", "password", config_override=fresh_config) + assert DEFAULT_BACKEND_HINT in str(info.value) + assert info.value.__cause__ is missing + + +def test_backend_argument_gets_no_hint(fresh_config): + """backend= passed to Connection counts as set: the original error is raised.""" + with pytest.raises(OSError, match="connection refused"): + _connect(fresh_config, backend="postgresql") + + +def test_backend_assigned_in_config_gets_no_hint(fresh_config): + """A backend assigned in config, even to the default value, counts as set.""" + fresh_config["database.backend"] = "postgresql" + with pytest.raises(OSError, match="connection refused"): + _connect(fresh_config) + + +def test_backend_from_env_var_gets_no_hint(monkeypatch): + """DJ_BACKEND counts as set.""" + monkeypatch.setenv("DJ_BACKEND", "postgresql") + with pytest.raises(OSError, match="connection refused"): + _connect(Config()) + + +def test_backend_from_config_file_gets_no_hint(fresh_config, tmp_path): + """database.backend in datajoint.json counts as set.""" + config_file = tmp_path / "datajoint.json" + config_file.write_text(json.dumps({"database": {"backend": "postgresql"}})) + fresh_config.load(config_file) + with pytest.raises(OSError, match="connection refused"): + _connect(fresh_config) + + +def test_refused_connection_with_default_backend(fresh_config): + """A real refused connection through the PostgreSQL adapter carries the hint.""" + pytest.importorskip("psycopg2") + # Find a local port with nothing listening on it. + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + port = sock.getsockname()[1] + with pytest.raises(DataJointError) as info: + Connection("127.0.0.1", "user", "password", port, use_tls=False, config_override=fresh_config) + assert DEFAULT_BACKEND_HINT in str(info.value) diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index a2fd26b3f..a2b28b4e3 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -164,7 +164,7 @@ def test_attribute_access(self): # Host can be localhost or db (docker), just verify it's a string assert isinstance(dj.config.database.host, str) assert len(dj.config.database.host) > 0 - # Port may be 3306 (default) or a random port (testcontainers) + # Port may be the backend's default or a random port (testcontainers) assert isinstance(dj.config.database.port, int) assert 1 <= dj.config.database.port <= 65535 # safemode may be modified by conftest fixtures @@ -175,7 +175,7 @@ def test_dict_style_access(self): # Host can be localhost or db (docker), just verify it's a string assert isinstance(dj.config["database.host"], str) assert len(dj.config["database.host"]) > 0 - # Port may be 3306 (default) or a random port (testcontainers) + # Port may be the backend's default or a random port (testcontainers) assert isinstance(dj.config["database.port"], int) assert 1 <= dj.config["database.port"] <= 65535 # safemode may be modified by conftest fixtures @@ -770,8 +770,9 @@ def test_save_minimal_template(self, tmp_path): content = json.load(f) assert "database" in content + assert content["database"]["backend"] == "postgresql" assert content["database"]["host"] == "localhost" - assert content["database"]["port"] == 3306 + assert content["database"]["port"] == 5432 # Minimal template should not have credentials assert "password" not in content["database"] assert "user" not in content["database"] @@ -1062,12 +1063,23 @@ def _isolate_dj_env(self, monkeypatch): monkeypatch.delenv(var, raising=False) def test_backend_default(self): - """Test default backend is mysql.""" + """Test default backend is postgresql on port 5432.""" from datajoint.settings import DatabaseSettings + settings = DatabaseSettings() + assert settings.backend == "postgresql" + assert settings.port == 5432 + assert "backend" not in settings.model_fields_set + + def test_backend_mysql_env_var(self, monkeypatch): + """DJ_BACKEND=mysql selects MySQL on port 3306 and counts as set explicitly.""" + from datajoint.settings import DatabaseSettings + + monkeypatch.setenv("DJ_BACKEND", "mysql") settings = DatabaseSettings() assert settings.backend == "mysql" assert settings.port == 3306 + assert "backend" in settings.model_fields_set def test_backend_postgresql(self, monkeypatch): """Test PostgreSQL backend with auto port.""" @@ -1121,7 +1133,6 @@ def test_config_file_backend(self, tmp_path, monkeypatch): from datajoint.settings import Config - # Include port in config since auto-detection only happens during initialization config_file = tmp_path / "test_config.json" config_file.write_text(json.dumps({"database": {"backend": "postgresql", "host": "db.example.com", "port": 5432}})) @@ -1138,7 +1149,7 @@ def test_config_file_backend(self, tmp_path, monkeypatch): def test_global_config_backend(self): """Test global config has backend configuration.""" - # Global config should have backend field with default mysql + # Global config has a backend field (conftest fixtures may have set it) assert hasattr(dj.config.database, "backend") # Backend should be one of the valid values assert dj.config.database.backend in ["mysql", "postgresql"] @@ -1146,19 +1157,59 @@ def test_global_config_backend(self): assert isinstance(dj.config.database.port, int) assert 1 <= dj.config.database.port <= 65535 - def test_port_auto_detection_on_initialization(self): - """Test port auto-detects only during initialization, not on live updates.""" + def test_port_follows_backend_when_not_set(self): + """A port that was not set follows changes of the backend on a live config.""" from datajoint.settings import DatabaseSettings - # Start with MySQL (default) settings = DatabaseSettings() - assert settings.port == 3306 + assert settings.port == 5432 - # Change backend on live config - port won't auto-update + settings.backend = "mysql" + assert settings.port == 3306 settings.backend = "postgresql" - # Port remains at previous value (this is expected behavior) - # Users should set port explicitly when changing backend on live config - assert settings.port == 3306 # Didn't auto-update + assert settings.port == 5432 + assert "port" not in settings.model_fields_set + + def test_explicit_port_survives_backend_change(self, monkeypatch): + """A port that was set, by assignment or DJ_PORT, is kept when the backend changes.""" + from datajoint.settings import DatabaseSettings + + settings = DatabaseSettings() + settings.port = 6543 + settings.backend = "mysql" + assert settings.port == 6543 + + monkeypatch.setenv("DJ_PORT", "3307") + settings = DatabaseSettings() + settings.backend = "mysql" + assert settings.port == 3307 + + def test_port_none_returns_to_backend_default(self): + """Setting the port to None makes it follow the backend again.""" + from datajoint.settings import DatabaseSettings + + settings = DatabaseSettings() + settings.port = 6543 + settings.port = None + assert settings.port == 5432 + settings.backend = "mysql" + assert settings.port == 3306 + + def test_config_file_mysql_backend_without_port(self, tmp_path): + """backend: mysql in datajoint.json, without a port, connects on port 3306.""" + import json + + from datajoint.settings import Config + + config_file = tmp_path / "datajoint.json" + config_file.write_text(json.dumps({"database": {"backend": "mysql", "host": "db.example.com"}})) + + cfg = Config() + assert cfg.database.port == 5432 + cfg.load(config_file) + assert cfg.database.backend == "mysql" + assert cfg.database.port == 3306 + assert "backend" in cfg.database.model_fields_set def test_mysql_backend_with_explicit_port(self, monkeypatch): """Test MySQL backend with explicit non-default port.""" diff --git a/tests/unit/test_thread_safe.py b/tests/unit/test_thread_safe.py index aba1b686b..f464f35b4 100644 --- a/tests/unit/test_thread_safe.py +++ b/tests/unit/test_thread_safe.py @@ -173,19 +173,26 @@ def test_instance_backend_sets_config(self, monkeypatch): ) assert inst.config.database.backend == "postgresql" - def test_instance_backend_default_from_config(self, monkeypatch): - """Instance without backend uses config default.""" + def test_instance_backend_default_from_config(self, monkeypatch, tmp_path): + """Instance without backend uses the config default, postgresql on port 5432.""" monkeypatch.setenv("DJ_THREAD_SAFE", "false") + monkeypatch.delenv("DJ_BACKEND", raising=False) + monkeypatch.delenv("DJ_PORT", raising=False) + # Keep a datajoint.json in a parent directory from supplying the backend + (tmp_path / ".git").mkdir() + monkeypatch.chdir(tmp_path) from datajoint.instance import Instance from unittest.mock import patch - with patch("datajoint.instance.Connection"): + with patch("datajoint.instance.Connection") as MockConn: inst = Instance( host="localhost", - user="root", + user="postgres", password="secret", ) - assert inst.config.database.backend == "mysql" + assert inst.config.database.backend == "postgresql" + args, kwargs = MockConn.call_args + assert args[3] == 5432 # port is the 4th positional arg def test_instance_backend_affects_port_default(self, monkeypatch): """Instance(backend='postgresql') uses port 5432 by default.""" @@ -204,6 +211,24 @@ def test_instance_backend_affects_port_default(self, monkeypatch): args, kwargs = MockConn.call_args assert args[3] == 5432 # port is the 4th positional arg + def test_instance_mysql_backend_uses_port_3306(self, monkeypatch): + """Instance(backend='mysql') uses port 3306 by default.""" + monkeypatch.setenv("DJ_THREAD_SAFE", "false") + monkeypatch.delenv("DJ_PORT", raising=False) + from datajoint.instance import Instance + from unittest.mock import patch + + with patch("datajoint.instance.Connection") as MockConn: + inst = Instance( + host="localhost", + user="root", + password="secret", + backend="mysql", + ) + assert inst.config.database.backend == "mysql" + args, kwargs = MockConn.call_args + assert args[3] == 3306 + class TestCrossConnectionValidation: """Test that cross-connection operations are rejected."""