From 3c4b2c43f4d8ad985a9277f3b73462761cfeac68 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Tue, 20 Oct 2026 03:37:09 +0200 Subject: [PATCH] feat: section name in config error messages extract_conn_params_from_section() named every section "section" in its errors, so plann reported a failing password_command as "of section section". It takes the real name as section_name now, and the test-server lookup passes it too. Prompt: Isn't the search-path decided by calendaring-config? Fix the finding #6. [the review finding that caldav's config errors say "of section section"] Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- caldav/config.py | 11 ++++++---- changes/+calendaring-config.changed.md | 2 +- tests/test_caldav_unit.py | 30 ++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/caldav/config.py b/caldav/config.py index 0bed39eb..f316f222 100644 --- a/caldav/config.py +++ b/caldav/config.py @@ -530,7 +530,9 @@ def _test_server_to_params(server: Any, was_already_started: bool) -> dict[str, def extract_conn_params_from_section( - section_data: dict[str, Any], config_file: str | None = None + section_data: dict[str, Any], + config_file: str | None = None, + section_name: str = "section", ) -> dict[str, Any] | None: """Extract connection parameters from a config section dict. @@ -541,7 +543,8 @@ def extract_conn_params_from_section( ``password_command`` / ``password_keyring`` are resolved, and a ``features`` key is resolved through :func:`resolve_features`. A ``password_command`` is only run when ``config_file`` names the file the - section came from. Public so that downstream tools (e.g. plann) can + section came from. ``section_name`` is the name error messages use. + Public so that downstream tools (e.g. plann) can reuse it on plann-style config sections, or on command line arguments: keys it does not know are ignored without a warning. @@ -552,7 +555,7 @@ def extract_conn_params_from_section( ``calendar_url``) are intentionally excluded — callers that need them read ``section_data`` directly. """ - conn = calendaring_config.section_connection("section", section_data, config_file) + conn = calendaring_config.section_connection(section_name, section_data, config_file) if conn.kind not in (None, *KINDS): return None params = _caldav_params(conn, warn=False) @@ -615,7 +618,7 @@ def get_all_test_servers( for section_name in cfg: section_data = config_section(cfg, section_name) if get_test_setting(section_data, "test_allowed"): - conn_params = extract_conn_params_from_section(section_data, str(path)) + conn_params = extract_conn_params_from_section(section_data, str(path), section_name) if conn_params: priority = get_test_setting(section_data, "test_priority") if priority is not None: diff --git a/changes/+calendaring-config.changed.md b/changes/+calendaring-config.changed.md index 6859005b..1bc8219d 100644 --- a/changes/+calendaring-config.changed.md +++ b/changes/+calendaring-config.changed.md @@ -4,7 +4,7 @@ The config file is now read through the [calendaring-config](https://github.com/ - A section may say `kind: caldav` and drop the `caldav_` prefix. Sections of other kinds (`kind: jmap`, a directory of `.ics` files) are skipped. - `password_command` runs a program for the password, and `password_keyring` looks it up in the system keyring. - `CALDAV_*` environment variables override the section's keys one by one: a `CALDAV_PASSWORD` alone no longer makes caldav ignore the config file (https://github.com/python-caldav/caldav/issues/780). `true`/`false` in a variable is a boolean now, and `CALDAV_HEADERS` is read as JSON. -- `extract_conn_params_from_section()`, which plann uses, reads the new format too; pass it the config file's path as `config_file` to let a `password_command` run. +- `extract_conn_params_from_section()`, which plann uses, reads the new format too; pass it the config file's path as `config_file` to let a `password_command` run, and the section's name as `section_name` for error messages. - An unknown key in a section is logged as a warning. `test_*`, `x_*` and `extra_config` are reserved for test tooling and applications. Some files are read differently: an empty file found first is used, rather than skipped for the next one; a section named directly that is missing or disabled, or a glob that matches nothing, is an error; a name in `contains` that matches nothing gives a warning; a disabled meta-section leaves out its members too; `*` is only the sections that are not meta-sections; and a config file named explicitly must exist. diff --git a/tests/test_caldav_unit.py b/tests/test_caldav_unit.py index e5a15fd9..2da4205f 100755 --- a/tests/test_caldav_unit.py +++ b/tests/test_caldav_unit.py @@ -5074,6 +5074,36 @@ def test_extract_reads_the_new_format(self, tmp_path, caplog): assert not caplog.records assert "features" in extract_conn_params_from_section({"features": "radicale"}) + def test_extract_names_the_section_in_errors(self, tmp_path): + import sys + + from caldav.config import extract_conn_params_from_section + + config_file = self._write(tmp_path, {}) + failing = {"caldav_url": "u", "caldav_password_command": [sys.executable, "-c", "exit(3)"]} + with pytest.raises(ValueError, match="section work "): + extract_conn_params_from_section(failing, config_file, section_name="work") + with pytest.raises(ValueError): + extract_conn_params_from_section(failing, config_file) + + def test_test_servers_name_the_section_in_errors(self, tmp_path): + import sys + + from caldav.config import get_all_test_servers + + config_file = self._write( + tmp_path, + { + "scratch": { + "caldav_url": "https://test.example.com/", + "caldav_password_command": [sys.executable, "-c", "exit(3)"], + "test_allowed": True, + } + }, + ) + with pytest.raises(ValueError, match="section scratch "): + get_all_test_servers(config_file) + def test_test_servers_in_the_new_format(self, tmp_path): import sys