diff --git a/admin/backend/api/v1/setup/config.py b/admin/backend/api/v1/setup/config.py index 0d99e33c6..b1f3df9a3 100644 --- a/admin/backend/api/v1/setup/config.py +++ b/admin/backend/api/v1/setup/config.py @@ -6,6 +6,7 @@ from admin.backend.api.v1.setup.database import local_database_credentials from admin.backend.api.v1.setup.state import setup_handoff_task from pilot.config import BenchConfig +from pilot.config.bench import get_framework_branch_error from pilot.internal.validators import validate_branch_name, validate_repo_url _PASSWORD_KEYS = ("admin_password", "mariadb_password", "postgres_password") @@ -36,6 +37,8 @@ def validate_configuration(data: dict) -> str | None: return error if error := _validate_existing_database(data): return error + if error := get_framework_branch_error(data.get("db_type", "mariadb"), data.get("app_branch", "")): + return error return _validate_app_source(data) diff --git a/admin/frontend/dashboard/src/composables/setup/useSetup.ts b/admin/frontend/dashboard/src/composables/setup/useSetup.ts index 6ca2a07b0..c8c75d082 100644 --- a/admin/frontend/dashboard/src/composables/setup/useSetup.ts +++ b/admin/frontend/dashboard/src/composables/setup/useSetup.ts @@ -24,6 +24,8 @@ const DB_TYPE_OPTIONS = [ { label: 'MariaDB', value: 'mariadb' }, { label: 'PostgreSQL', value: 'postgres' }, ] +// Frappe supports PostgreSQL only on develop. +const POSTGRES_FRAMEWORK_BRANCH = 'develop' const STEP_TITLES: Record = { database: 'Database', customize: 'Customize your bench', @@ -95,6 +97,10 @@ export const useSetup = () => { const dbPortPlaceholder = computed(() => (dbType.value === 'mariadb' ? '3306' : '5432')) const resolvedDbUser = computed(() => dbUser.value || rootUserPlaceholder.value) + const isBranchLocked = computed(() => dbType.value === 'postgres') + watch(isBranchLocked, (locked) => { + if (locked) appBranch.value = POSTGRES_FRAMEWORK_BRANCH + }) const branchOptions = computed(() => branchComboboxOptions(availableBranches.value, appBranch.value, (typed: string) => { appBranch.value = typed @@ -427,6 +433,7 @@ export const useSetup = () => { rootUserPlaceholder, dbTypeOptions: DB_TYPE_OPTIONS, branchOptions, + isBranchLocked, validatingFramework, frameworkIsValid, stepSequence, diff --git a/admin/frontend/dashboard/src/pages/setup/Setup.vue b/admin/frontend/dashboard/src/pages/setup/Setup.vue index f4e4aaf69..2603d73c5 100644 --- a/admin/frontend/dashboard/src/pages/setup/Setup.vue +++ b/admin/frontend/dashboard/src/pages/setup/Setup.vue @@ -26,6 +26,7 @@ const { rootUserPlaceholder, dbTypeOptions, branchOptions, + isBranchLocked, validatingFramework, frameworkIsValid, stepSequence, @@ -107,6 +108,8 @@ const { label="Frappe branch" v-model="appBranch" :options="branchOptions" + :disabled="isBranchLocked" + :description="isBranchLocked ? 'PostgreSQL needs the develop branch.' : undefined" trigger="button" placeholder="Search or type a branch…" > diff --git a/docs/admin-ui.md b/docs/admin-ui.md index 89adf87cf..05c44226d 100644 --- a/docs/admin-ui.md +++ b/docs/admin-ui.md @@ -55,7 +55,7 @@ The UI should treat task ids as the handle for long work. ## Setup -The Frappe branch picker lists the branches reported by the repository. Type a different full branch name and select `Use branch` to choose it. `Set up bench` saves the selection before setup starts. The wizard restores a saved custom branch even when the repository list does not contain it. +The Frappe branch picker lists the branches reported by the repository. Type a different full branch name and select `Use branch` to choose it. `Set up bench` saves the selection before setup starts. The wizard restores a saved custom branch even when the repository list does not contain it. Choosing PostgreSQL locks the picker to `develop`. ## Site Navigation diff --git a/docs/configuration.md b/docs/configuration.md index 5409e89ce..ccfa06641 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -32,7 +32,7 @@ queues = ["default", "short", "long"] count = 1 ``` -The first app is the framework app. A bench uses one database type for all its sites. Supported types are `mariadb` and `postgres`. +The first app is the framework app. A bench uses one database type for all its sites. Supported types are `mariadb` and `postgres`. A `postgres` bench needs the framework app on `develop`; `pilot init` and setup reject other branches. ## Bench settings diff --git a/pilot/config/bench.py b/pilot/config/bench.py index b8f4920bc..93e6fe241 100644 --- a/pilot/config/bench.py +++ b/pilot/config/bench.py @@ -84,6 +84,14 @@ # Framework branches the setup wizard offers, newest/recommended first. FRAMEWORK_BRANCHES = ["version-16", "develop"] +# Frappe supports PostgreSQL only on develop. +POSTGRES_FRAMEWORK_BRANCH = "develop" + + +def get_framework_branch_error(db_type: str, branch: str) -> str | None: + if db_type == "postgres" and branch != POSTGRES_FRAMEWORK_BRANCH: + return f"PostgreSQL benches need Frappe branch '{POSTGRES_FRAMEWORK_BRANCH}', not '{branch}'." + return None _DEFAULT_DATA: dict = { "bench": {"name": "", "python": "3.14"}, diff --git a/pilot/core/bench/creator.py b/pilot/core/bench/creator.py index ca080d001..bb2dd44f2 100644 --- a/pilot/core/bench/creator.py +++ b/pilot/core/bench/creator.py @@ -5,6 +5,7 @@ from pathlib import Path from typing import TYPE_CHECKING +from pilot.config.bench import POSTGRES_FRAMEWORK_BRANCH from pilot.exceptions import BenchAlreadyExistsError from pilot.utils import iter_sibling_benches @@ -80,6 +81,8 @@ def _initial_settings(self) -> dict: "db_type": self.db_type, "lite_mode_enabled": True, } + if self.db_type == "postgres": + settings["app_branch"] = POSTGRES_FRAMEWORK_BRANCH if self.process_manager: settings["production_process_manager"] = self.process_manager return settings diff --git a/pilot/core/bench/initializer.py b/pilot/core/bench/initializer.py index 74b4352a1..2ec824a1b 100644 --- a/pilot/core/bench/initializer.py +++ b/pilot/core/bench/initializer.py @@ -4,7 +4,9 @@ from collections.abc import Callable from typing import TYPE_CHECKING +from pilot.config.bench import get_framework_branch_error from pilot.config.common_site_config import update_common_site_config +from pilot.exceptions import ConfigError if TYPE_CHECKING: from pilot.core.bench import Bench @@ -40,7 +42,7 @@ def _run_steps(self, on_progress: Callable[[str], None]) -> None: # by install.sh, and MariaDB/PostgreSQL run as a rootless per-user # server (see MariaDBManager/PostgresManager). steps: list[tuple[str, Callable[[], None]]] = [ - ("Validate bench.toml", self.bench.config.validate), + ("Validate bench.toml", self._validate_config), ("Ensure admin password", self._ensure_admin_password), ("Ensure database credentials", self._ensure_database_credentials), ("Install system packages", self._install_system_packages), @@ -67,6 +69,12 @@ def _run_steps(self, on_progress: Callable[[str], None]) -> None: on_progress(" pilot new-site site1.example.com # create your first site") on_progress(" pilot start # start all processes") + def _validate_config(self) -> None: + config = self.bench.config + config.validate() + if error := get_framework_branch_error(config.db_type, config.framework_app.branch): + raise ConfigError(error) + def _rollback(self, on_progress: Callable[[str], None]) -> None: if not self._rollback_actions: return diff --git a/tests/admin/backend/api/test_setup_postgres.py b/tests/admin/backend/api/test_setup_postgres.py index 6e88f06c4..b82c2f675 100644 --- a/tests/admin/backend/api/test_setup_postgres.py +++ b/tests/admin/backend/api/test_setup_postgres.py @@ -13,10 +13,16 @@ def test_validate_configuration_requires_postgres_password() -> None: def test_validate_configuration_accepts_postgres_password() -> None: - data = {"admin_password": "x", "db_type": "postgres", "postgres_password": "pw"} + data = {"admin_password": "x", "db_type": "postgres", "postgres_password": "pw", "app_branch": "develop"} assert validate_configuration(data) is None +def test_validate_configuration_rejects_postgres_off_develop() -> None: + data = {"admin_password": "x", "db_type": "postgres", "postgres_password": "pw", "app_branch": "version-16"} + error = validate_configuration(data) + assert error and "develop" in error + + def test_validate_configuration_requires_mariadb_password() -> None: error = validate_configuration({"admin_password": "x", "db_type": "mariadb", "mariadb_password": ""}) assert error and "mariadb_password" in error diff --git a/tests/e2e/flows/wizard.py b/tests/e2e/flows/wizard.py index f1ddf0a1a..a31f77f65 100644 --- a/tests/e2e/flows/wizard.py +++ b/tests/e2e/flows/wizard.py @@ -37,7 +37,9 @@ def complete_dev_wizard( # choice - that's a separate `pilot setup production` step run from the terminal # afterwards). We keep the repo default. expect(page.get_by_text("Customize your bench")).to_be_visible(timeout=30_000) - if framework_branch: + if db_type == "postgres": + _expect_branch_locked(page) + elif framework_branch: _choose_combobox_button(page, "Frappe branch", framework_branch) page.get_by_role("button", name="Set up bench").click() @@ -80,6 +82,13 @@ def _choose_select(page: Page, label: str, option_name: str) -> None: page.get_by_role("option", name=option_name).click() +def _expect_branch_locked(page: Page) -> None: + """PostgreSQL benches are locked to the develop branch.""" + picker = page.get_by_role("button", name="Frappe branch") + expect(picker).to_be_disabled() + expect(picker).to_contain_text("develop") + + def _choose_combobox_button(page: Page, label: str, option_name: str) -> None: """Choose an option in a Frappe UI Combobox with `trigger="button"`.""" page.get_by_role("button", name=label).click() diff --git a/tests/pilot/commands/test_commands.py b/tests/pilot/commands/test_commands.py index bd53111f8..bea0f9cbc 100644 --- a/tests/pilot/commands/test_commands.py +++ b/tests/pilot/commands/test_commands.py @@ -178,6 +178,7 @@ def test_new_command_postgres_bench_has_no_password_yet( with open(benches_dir / "pg" / "bench.toml", "rb") as f: data = tomllib.load(f) assert data["bench"]["db_type"] == "postgres" + assert data["apps"][0]["branch"] == "develop" assert not CommonConfig.read(benches_dir).postgres.root_password diff --git a/tests/pilot/commands/test_init_command.py b/tests/pilot/commands/test_init_command.py index 9afa103fd..9c880d4c2 100644 --- a/tests/pilot/commands/test_init_command.py +++ b/tests/pilot/commands/test_init_command.py @@ -54,6 +54,15 @@ def test_raises_when_existing_credentials_are_wrong() -> None: manager.provision.assert_not_called() +def test_rejects_postgres_bench_off_develop() -> None: + bench = MagicMock() + bench.config.db_type = "postgres" + bench.config.framework_app.branch = "version-16" + + with pytest.raises(BenchError, match="develop"): + BenchInitializer(bench)._validate_config() + + def test_ensure_database_credentials_generates_for_a_fresh_server() -> None: bench = MagicMock() bench.config.db_type = "mariadb"