From 11deca40f42e36d0ab28c1e2cf602f7902d05f9c Mon Sep 17 00:00:00 2001 From: John Barker Date: Thu, 5 Nov 2026 09:24:17 +0000 Subject: [PATCH 1/2] feat: Improved provisioning logging. As the number of Cities increase it's getting difficult to check the status Clearly show: * OK * Updated * Error * Manual check needed --- tests/test_ansible_events_lib.py | 64 ++++++++++++- tools/common/ansible_events_lib.py | 6 +- tools/environment/provision_environment.py | 105 ++++++++++++++++----- 3 files changed, 144 insertions(+), 31 deletions(-) diff --git a/tests/test_ansible_events_lib.py b/tests/test_ansible_events_lib.py index f144d48..546556e 100644 --- a/tests/test_ansible_events_lib.py +++ b/tests/test_ansible_events_lib.py @@ -306,7 +306,7 @@ def test_discourse_request_uses_explicit_run_as_identity(self, monkeypatch): assert ansible_events_lib.DISCOURSE_CLIENT.session.headers["Api-Key"] == "test-api-key" assert request.call_args.kwargs["headers"]["Api-Username"] == "organiser" - def test_discourse_request_retries_rate_limit_using_wait_seconds(self, monkeypatch): + def test_discourse_request_retries_rate_limit_using_wait_seconds(self, monkeypatch, caplog): from tools.common import ansible_events_lib monkeypatch.setattr(ansible_events_lib, "DISCOURSE_API_KEY", "test-api-key") @@ -323,6 +323,8 @@ def test_discourse_request_retries_rate_limit_using_wait_seconds(self, monkeypat assert request.call_count == 2 sleep.assert_called_once_with(31.0) + warning = next(record for record in caplog.records if record.levelname == "WARNING") + assert "Discourse rate limit for GET c/43/show.json" in warning.message def test_discourse_request_rate_limit_retries_are_bounded(self, monkeypatch): from tools.common import ansible_events_lib @@ -725,7 +727,9 @@ def unexpected_request(*_args, **_kwargs): ] ) - def test_pretix_teams_do_not_require_pretix_two_factor_authentication(self, monkeypatch): + def test_pretix_teams_do_not_require_pretix_two_factor_authentication(self, monkeypatch, caplog): + import logging + from tools.environment import provision_environment requests = [] @@ -753,12 +757,17 @@ def request(method, endpoint, payload=None): monkeypatch.setattr(provision_environment, "pretix_list_all", list_all) monkeypatch.setattr(provision_environment, "pretix_req", request) + caplog.set_level(logging.INFO, logger=provision_environment.logger.name) provision_environment.reconcile_pretix_teams() team_creations = [payload for method, endpoint, payload in requests if method == "POST" and endpoint == "teams"] assert len(team_creations) == len(provision_environment.CITIES) + 1 assert all(team["require_2fa"] is False for team in team_creations) + messages = [record.getMessage() for record in caplog.records] + for city in provision_environment.CITIES: + assert city.city in messages + assert f" 🟡 {city.team_name}: Created" in messages def test_template_ticket_reconciles_item_and_quota_drift(self, monkeypatch): from tools.environment import provision_environment @@ -903,13 +912,17 @@ def test_resource_status_lists_changed_and_already_correct_fields(self, caplog): from tools.environment import provision_environment caplog.set_level(logging.INFO, logger=provision_environment.logger.name) + provision_environment.set_log_scope("Test City") provision_environment.log_resource_status( "meetup-organisers-london", {"visibility_level": 1, "members_visibility_level": 2}, {"visibility_level": 0, "members_visibility_level": 2}, ) - assert "meetup-organisers-london: Updated | visibility: Changed, member visibility: OK" in caplog.text + messages = [record.getMessage() for record in caplog.records] + assert " 🟡 meetup-organisers-london: Updated" in messages + assert " 🟡 visibility: Updated" in messages + assert " ✅ member visibility: Already correct" in messages def test_resource_status_marks_created_fields_as_verified(self, caplog): import logging @@ -917,15 +930,52 @@ def test_resource_status_marks_created_fields_as_verified(self, caplog): from tools.environment import provision_environment caplog.set_level(logging.INFO, logger=provision_environment.logger.name) + provision_environment.set_log_scope("Test City") provision_environment.log_resource_status( "meetup-attendee-london", {"visibility_level": 3, "members_visibility_level": 3}, None, ) - assert "meetup-attendee-london: Created | visibility: OK, member visibility: OK" in caplog.text + messages = [record.getMessage() for record in caplog.records] + assert " 🟡 meetup-attendee-london: Created" in messages + assert " ✅ visibility: Already correct" in messages + assert " ✅ member visibility: Already correct" in messages + + def test_resource_status_marks_manual_fields_for_follow_up(self, caplog): + import logging + + from tools.environment import provision_environment + + caplog.set_level(logging.INFO, logger=provision_environment.logger.name) + provision_environment.set_log_scope("Manual City") + provision_environment.log_resource_status( + "meetup-attendee-manual-city", + {"description": "desired"}, + {"description": "custom"}, + {"description": "OK:MANUAL"}, + ) + + assert " ❓ description: Manual follow-up" in [record.getMessage() for record in caplog.records] + + def test_provisioning_api_failure_has_failure_marker(self, caplog): + import logging + + from tools.common import ansible_events_lib + + caplog.set_level(logging.ERROR, logger=ansible_events_lib.logger.name) + + def fail(): + raise ansible_events_lib.ApiError("test failure") + + with pytest.raises(SystemExit): + ansible_events_lib.run_cli(fail) + + assert "❌ API request failed: test failure" in caplog.text + + def test_city_category_uses_reply_only_default_and_city_organiser_access(self, monkeypatch, caplog): + import logging - def test_city_category_uses_reply_only_default_and_city_organiser_access(self, monkeypatch): from tools.environment import provision_environment from tools.common.ansible_events_lib import get_city @@ -961,6 +1011,7 @@ def request(method, endpoint, payload=None): raise AssertionError(f"Unexpected Discourse request: {method} {endpoint}") monkeypatch.setattr(provision_environment, "discourse_req", request) + caplog.set_level(logging.INFO, logger=provision_environment.logger.name) provision_environment.reconcile_city_category_access( [current.copy()], {51: plan}, @@ -974,6 +1025,9 @@ def request(method, endpoint, payload=None): } assert update["moderating_group_ids"] == [9] assert sum(method == "GET" for method, _, _ in requests) == 2 + messages = [record.getMessage() for record in caplog.records] + assert "London" in messages + assert " 🟡 Events > London: Updated" in messages def test_organizer_access_is_removed_everywhere_except_own_subcategory(self, monkeypatch): from tools.environment import provision_environment diff --git a/tools/common/ansible_events_lib.py b/tools/common/ansible_events_lib.py index ccce390..7a30aa8 100644 --- a/tools/common/ansible_events_lib.py +++ b/tools/common/ansible_events_lib.py @@ -28,7 +28,7 @@ def run_cli(action: Callable[[], None]) -> None: try: action() except ApiError as exc: - logger.error("API request failed: %s", exc) + logger.error("❌ API request failed: %s", exc) raise SystemExit(1) from exc @@ -318,10 +318,10 @@ def pre_flight_checks(*, require_discourse: bool = True, require_pretix: bool = if require_pretix: parsed = urlparse(PRETIX_URL) if parsed.scheme != "https" and parsed.hostname not in {"localhost", "127.0.0.1", "::1"}: - logger.error("PRETIX_URL must use HTTPS outside localhost/loopback") + logger.error("❌ PRETIX_URL must use HTTPS outside localhost/loopback") missing.append("secure PRETIX_URL") if missing: - logger.error("Missing required environment variables: %s", ", ".join(missing)) + logger.error("❌ Missing required environment variables: %s", ", ".join(missing)) sys.exit(1) diff --git a/tools/environment/provision_environment.py b/tools/environment/provision_environment.py index 3b8a0b4..3da205b 100755 --- a/tools/environment/provision_environment.py +++ b/tools/environment/provision_environment.py @@ -53,6 +53,28 @@ INVITE_PENDING_ENDPOINT = f"u/{INVITE_API_USERNAME}/invited/pending.json" INVITE_MAX_REDEMPTIONS = 5_000 INVITE_EXPIRES_AT = "2030-01-01" +_LOG_INDENT = 0 +_LOG_SCOPE: str | None = None + + +def set_log_scope(name: str) -> None: + """Start a top-level global or city section in provisioning output.""" + global _LOG_INDENT, _LOG_SCOPE + _LOG_INDENT = 0 + if name != _LOG_SCOPE: + logger.info("%s", name) + _LOG_SCOPE = name + _LOG_INDENT = 2 + + +def log_detail(message: str, *args: Any) -> None: + """Log an informational detail under the active global or city section.""" + logger.info("%s" + message, " " * _LOG_INDENT, *args) + + +def log_check(message: str, *args: Any) -> None: + """Log an individual field check nested under its resource.""" + logger.info("%s " + message, " " * _LOG_INDENT, *args) @dataclass(slots=True) @@ -170,6 +192,8 @@ def pending_invites_by_description(response: Mapping[str, Any]) -> set[tuple[str def reconcile_attendee_invites() -> None: """Create missing city invite links for attendee and migration groups.""" + set_log_scope("Global") + log_detail("Reconciling attendee invites...") response = discourse_req("GET", INVITE_PENDING_ENDPOINT, run_as=INVITE_API_USERNAME) invite_targets = pending_invites_by_description(response) @@ -180,7 +204,8 @@ def reconcile_attendee_invites() -> None: invite_description == description and set(target_groups).issubset(group_names) for invite_description, group_names in invite_targets ): - logger.info("Invite for %s already exists", city.city) + set_log_scope(city.city) + log_detail("✅ Invite already exists") continue payload = { @@ -191,7 +216,8 @@ def reconcile_attendee_invites() -> None: "expires_at": INVITE_EXPIRES_AT, } created = discourse_req("POST", "invites.json", payload, run_as=INVITE_API_USERNAME) - logger.info("Created invite for %s: %s", city.city, created.get("link", "link unavailable")) + set_log_scope(city.city) + log_detail("🟡 Created invite: %s", created.get("link", "link unavailable")) def list_discourse_groups() -> list[dict]: @@ -315,8 +341,22 @@ def log_resource_status( state = "Updated" else: state = "Already configured" - details = ", ".join(f"{field}: {status}" for field, status in statuses.items()) - logger.info("%s: %s | %s", resource, state, details) + marker = "🟡" if state in {"Created", "Updated"} else "✅" + log_detail("%s %s: %s", marker, resource, state) + for field, status in statuses.items(): + if status == "OK": + marker = "✅" + label = "Already correct" + elif status == "Changed": + marker = "🟡" + label = "Updated" + elif status.endswith(":MANUAL"): + marker = "❓" + label = "Manual follow-up" + else: + marker = "❌" + label = status + log_check("%s %s: %s", marker, field, label) def is_pretix_choice_error(error: ApiError, field: str) -> bool: @@ -378,7 +418,8 @@ def reconcile_pretix_event_settings( before, ) for field, status in preflight_status.items(): - logger.info("Pretix event %s setting: %s", field.replace("_", " "), status) + marker = "✅" if status == "Already configured" else "🟡" + log_check("%s Pretix event setting: %s", marker, field.replace("_", " ")) return after = pretix_req("PATCH", endpoint, changes) @@ -395,7 +436,8 @@ def reconcile_pretix_event_settings( before, ) for field, status in preflight_status.items(): - logger.info("Pretix event %s setting: %s", field.replace("_", " "), status) + marker = "✅" if status == "Already configured" else "🟡" + log_check("%s Pretix event setting: %s", marker, field.replace("_", " ")) def reconcile_pretix_mapping( @@ -437,7 +479,7 @@ def reconcile_pretix_teams() -> list[dict[str, Any]]: raise ApiError("Pretix teams response contains a team without a valid name") teams_by_name.setdefault(name, []).append(team) - desired_teams = [ + desired_teams: list[tuple[str, dict[str, Any], str | None]] = [ ( STAFF_TEAM_NAME, { @@ -450,6 +492,7 @@ def reconcile_pretix_teams() -> list[dict[str, Any]]: "all_organizer_permissions": False, "limit_organizer_permissions": [], }, + None, ) ] for city in CITIES: @@ -480,10 +523,12 @@ def reconcile_pretix_teams() -> list[dict[str, Any]]: "limit_organizer_permissions": [], "limit_events": city_events, }, + city.city, ) ) - for name, desired in desired_teams: + for name, desired, city_name in desired_teams: + set_log_scope(city_name or "Global") existing = teams_by_name.get(name, []) if len(existing) > 1: raise ApiError(f"Multiple Pretix teams are named {name!r}") @@ -510,6 +555,7 @@ def reconcile_pretix_teams() -> list[dict[str, Any]]: def reconcile_pretix_meta_property() -> None: """Ensure the Forum event URL property exists and cannot be edited by organisers.""" + set_log_scope("Global") properties = pretix_list_all("event_meta_properties") if any(not isinstance(prop.get("name"), str) for prop in properties): raise ApiError("Pretix event metadata response contains a property without a valid name") @@ -622,6 +668,7 @@ def reconcile_template_item_meta_property() -> None: def reconcile_pretix_template(events: list[dict[str, Any]]) -> None: """Create or reconcile the private clone source and its settings/ticket configuration.""" + set_log_scope("Global") matches = [event for event in events if event.get("slug") == TEMPLATE_SLUG] if len(matches) > 1: raise ApiError(f"Pretix has multiple events named {TEMPLATE_SLUG!r}") @@ -634,7 +681,7 @@ def reconcile_pretix_template(events: list[dict[str, Any]]) -> None: event = pretix_req("GET", f"events/{TEMPLATE_SLUG}") reconcile_pretix_mapping(f"events/{TEMPLATE_SLUG}", TEMPLATE_SLUG, desired_event, event) else: - logger.info("Creating new master template (%s)...", TEMPLATE_SLUG) + log_detail("🟡 Creating new master template (%s)...", TEMPLATE_SLUG) event = pretix_req( "POST", "events", @@ -650,7 +697,7 @@ def reconcile_pretix_template(events: list[dict[str, Any]]) -> None: raise ApiError(f"Pretix template {TEMPLATE_SLUG!r} did not reconcile after creation") log_resource_status(TEMPLATE_SLUG, desired_event, None) - logger.info("Reconciling Pretix template settings...") + log_detail("Reconciling Pretix template settings...") reconcile_pretix_event_settings( TEMPLATE_SLUG, { @@ -881,6 +928,8 @@ def reconcile_city_category_access( if not isinstance(category_id, int): raise ApiError("Discourse categories response contains an invalid category ID") checked_categories += 1 + plan = plans_by_category_id.get(category_id) + set_log_scope(plan.city.city if plan is not None else "Global") details_response = discourse_req("GET", f"c/{category_id}/show.json") current = details_response.get("category", details_response) if not isinstance(current, dict): @@ -890,7 +939,6 @@ def reconcile_city_category_access( if not isinstance(current_ids, list) or any(not isinstance(group_id, int) for group_id in current_ids): raise ApiError(f"Discourse category {category_id} returned invalid moderator group IDs") current_permissions = category_permission_map(current, category_id) - plan = plans_by_category_id.get(category_id) city_settings = plan.category_settings() if plan is not None else {} if plan is not None: @@ -947,6 +995,7 @@ def reconcile_city_category_access( updated_categories.append(f"{current.get('name', category_id)} ({category_id})") if plan is not None: + set_log_scope(plan.city.city) log_resource_status( f"Events > {plan.city.city}", city_settings, @@ -954,14 +1003,16 @@ def reconcile_city_category_access( ) if updated_categories: - logger.info( - "Discourse organiser access: checked %d categories; updated %d: %s", + set_log_scope("Global") + log_detail( + "🟡 Discourse organiser access: checked %d categories; updated %d: %s", checked_categories, len(updated_categories), ", ".join(updated_categories), ) else: - logger.info("Discourse organiser access: checked %d categories; no changes required", checked_categories) + set_log_scope("Global") + log_detail("✅ Discourse organiser access: checked %d categories; no changes required", checked_categories) def ensure_category_group_moderation_enabled() -> None: @@ -983,7 +1034,8 @@ def ensure_category_group_moderation_enabled() -> None: def ensure_user_field_options() -> None: """Add any missing city options to the user_event_cities custom field (additive only).""" - logger.info("Syncing user field options for city subscriptions...") + set_log_scope("Global") + log_detail("Syncing user field options for city subscriptions...") # GET uses hyphenated path, PUT uses underscored path (Discourse routing) resp = discourse_req("GET", f"admin/config/user-fields/{USER_CITY_FIELD_ID}.json") @@ -995,11 +1047,11 @@ def ensure_user_field_options() -> None: missing = desired_options - current_options if not missing: - logger.info("User field options up to date (%d options)", len(current_options)) + log_detail("✅ User field options up to date (%d options)", len(current_options)) return updated_options = sorted(current_options | desired_options) - logger.info("Adding %d new option(s): %s", len(missing), ", ".join(sorted(missing))) + log_detail("🟡 Adding %d new option(s): %s", len(missing), ", ".join(sorted(missing))) discourse_req( "PUT", @@ -1010,6 +1062,7 @@ def ensure_user_field_options() -> None: def reconcile_pretix_global_settings() -> None: """Reconcile Pretix organizer-wide settings.""" + set_log_scope("Global") organizer_settings = { "primary_color": ANSIBLE_PRIMARY_COLOR, "imprint_url": {"en": CODE_OF_CONDUCT_URL}, @@ -1040,7 +1093,7 @@ def reconcile_discourse_environment(groups_by_name: dict[str, dict[str, Any]]) - categories = discourse_categories() plans = build_city_forum_plans() - logger.info("Ensuring group %s...", MIGRATED_GROUP_NAME) + set_log_scope("Global") migrated_settings = { "name": MIGRATED_GROUP_NAME, "full_name": "Ansible Meetup - Migrated from Meetup Pro", @@ -1086,6 +1139,7 @@ def reconcile_discourse_environment(groups_by_name: dict[str, dict[str, Any]]) - plan.category_id = matches[0]["id"] for plan in plans: + set_log_scope(plan.city.city) if plan.organiser_group_id is None or plan.category_id is None: raise ApiError(f"Could not resolve Discourse resources for city {plan.city.slug!r}") reconcile_discourse_group( @@ -1115,14 +1169,17 @@ def reconcile_discourse_environment(groups_by_name: dict[str, dict[str, Any]]) - def reconcile_pretix() -> None: """Reconcile Pretix global settings, teams, metadata, and template.""" pre_flight_checks(require_discourse=False, require_pretix=True) - logger.info("--- PRETIX PROVISIONING ---") + set_log_scope("Global") + log_detail("PRETIX PROVISIONING") reconcile_pretix_global_settings() - logger.info("Reconciling Pretix teams and metadata...") + set_log_scope("Global") + log_detail("Reconciling Pretix teams and metadata...") all_events = reconcile_pretix_teams() reconcile_pretix_meta_property() - logger.info("Reconciling Pretix template...") + set_log_scope("Global") + log_detail("Reconciling Pretix template...") reconcile_pretix_template(all_events) @@ -1131,7 +1188,8 @@ def reconcile_forum() -> None: pre_flight_checks(require_discourse=True, require_pretix=False) # Validate the complete meetup group inventory before making any changes. groups_by_name = index_groups_by_name(list_discourse_groups()) - logger.info("--- FORUM PROVISIONING ---") + set_log_scope("Global") + log_detail("FORUM PROVISIONING") reconcile_discourse_environment(groups_by_name) @@ -1151,7 +1209,8 @@ def main() -> None: if args.forum: reconcile_forum() - logger.info("--- PROVISIONING COMPLETE ---") + set_log_scope("Global") + log_detail("PROVISIONING COMPLETE") if __name__ == "__main__": From 0f6303d77719e7f2f2cba1e70bdcba042f7318b7 Mon Sep 17 00:00:00 2001 From: John Barker Date: Thu, 5 Nov 2026 09:53:01 +0000 Subject: [PATCH 2/2] Create events, don't post on Forum by default --- tests/test_check_setup.py | 22 ++++++++++++++---- tools/environment/check_setup.py | 40 ++++++++++++++++++++++++++------ 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/tests/test_check_setup.py b/tests/test_check_setup.py index 67722de..6f152c4 100644 --- a/tests/test_check_setup.py +++ b/tests/test_check_setup.py @@ -203,15 +203,27 @@ def test_local_configuration_never_displays_secret_values(monkeypatch: pytest.Mo assert "forum-secret" not in checker.results[0].detail -def test_print_results_fails_only_for_fail_status(capsys: pytest.CaptureFixture[str]) -> None: +def test_print_results_uses_hierarchical_status_markers_and_fails_only_on_fail( + capsys: pytest.CaptureFixture[str], +) -> None: manual = check_setup.CheckResult("MANUAL", "server check", "operator", "unavailable", "inspect") + city = check_setup.CheckResult( + "PASS", + f"Forum/Pretix mapping: {CITIES[0].city}", + f"{CITIES[0].organiser_group} → {CITIES[0].team_name}", + "mapping is correct", + "None.", + ) failure = check_setup.CheckResult("FAIL", "resource", "GET settings", "missing", "reconcile") - assert check_setup.print_results([manual]) is False + assert check_setup.print_results([manual, city]) is False manual_output = capsys.readouterr().out assert "1 MANUAL" in manual_output - assert "🔴" not in manual_output - assert check_setup.print_results([manual, failure]) is True + assert "Global\n ❓ MANUAL: server check" in manual_output + assert f"{CITIES[0].city}\n ✅ PASS: Forum/Pretix mapping: {CITIES[0].city}" in manual_output + assert " Target: operator" in manual_output + assert "❌ FAIL:" not in manual_output + assert check_setup.print_results([manual, city, failure]) is True failed_output = capsys.readouterr().out - assert "🔴 FAIL: resource" in failed_output + assert " ❌ FAIL: resource" in failed_output assert "1 FAIL" in failed_output diff --git a/tools/environment/check_setup.py b/tools/environment/check_setup.py index 3d3c885..f62be0b 100644 --- a/tools/environment/check_setup.py +++ b/tools/environment/check_setup.py @@ -775,16 +775,42 @@ def run(self) -> list[CheckResult]: return self.results +def result_scope(result: CheckResult) -> str: + """Return the city section for a city-specific check, or Global.""" + searchable = f"{result.name} {result.target}".casefold() + for city in CITIES: + if any( + identifier.casefold() in searchable + for identifier in (city.city, city.slug, city.organiser_group, city.team_name) + ): + return city.city + return "Global" + + def print_results(results: list[CheckResult]) -> bool: - """Print sanitized results and return whether any diagnostic failed.""" + """Print sanitized, hierarchically grouped results and report whether any failed.""" + grouped: dict[str, list[CheckResult]] = {"Global": []} + grouped.update({city.city: [] for city in CITIES}) for result in results: - marker = "🔴 " if result.status == "FAIL" else "" - print(f"{marker}{result.status}: {result.name}") - print(f" Target: {result.target}") - print(f" Result: {result.detail}") - print(f" Next: {result.next_step}") + grouped[result_scope(result)].append(result) + + for scope in ("Global", *(city.city for city in CITIES)): + scoped_results = grouped[scope] + if not scoped_results: + continue + print(scope) + for result in scoped_results: + marker = {"PASS": "✅", "FAIL": "❌", "MANUAL": "❓"}.get(result.status, "❌") + print(f" {marker} {result.status}: {result.name}") + print(f" Target: {result.target}") + print(f" Result: {result.detail}") + print(f" Next: {result.next_step}") counts = {status: sum(result.status == status for result in results) for status in ("PASS", "FAIL", "MANUAL")} - print(f"Summary: {counts['PASS']} PASS, {counts['FAIL']} FAIL, {counts['MANUAL']} MANUAL") + print( + f"Summary: ✅ {counts['PASS']} PASS, " + f"{('❌ ' if counts['FAIL'] else '')}{counts['FAIL']} FAIL, " + f"❓ {counts['MANUAL']} MANUAL" + ) return counts["FAIL"] > 0