diff --git a/docs/development.md b/docs/development.md index d70ad39..4fb9be3 100644 --- a/docs/development.md +++ b/docs/development.md @@ -339,7 +339,11 @@ the `--verbose`/`--debug` flags: milestone messages (e.g. "Logging in to ...", "Login completed: ..."). - `--debug`: the logger is raised to `DEBUG`, which additionally enables HTTP-level tracing from `src/py_moodle/auth.py` (requests, status codes, - response URLs). + response URLs) and from the centralized HTTP layer + `src/py_moodle/http.py` (the `py_moodle.http` logger traces every request + it sends as `HTTP -> `, including retry + attempts). Only the method, the **redacted** URL, and the response status + are ever traced — never params, request/response bodies, or headers. All diagnostic output is written to stderr (never stdout), so it never mixes with the machine-readable payload of `--output json`/`csv`. diff --git a/docs/recipes.md b/docs/recipes.md index aa9180c..dbe10a0 100644 --- a/docs/recipes.md +++ b/docs/recipes.md @@ -91,6 +91,35 @@ py-moodle courses list --output csv > courses.csv py-moodle users list --course-id 2 --output csv | cut -d, -f3 ``` +## Select only the fields you need with `--fields` + +The `courses`, `categories`, `sections`, and `users` `list` commands accept +`--fields field1,field2,...` to project the machine-readable output +(`--output json`/`yaml`/`csv`) down to just the fields you want, in the order +you list them. This avoids a post-processing step with `jq`/`cut` and gives +you a stable, minimal shape for automation. + +```bash +# Only the id and shortname of every course, as JSON, in that order. +py-moodle courses list --output json --fields id,shortname + +# The same projection as CSV (the selected fields become the columns). +py-moodle courses list --output csv --fields id,shortname > courses.csv + +# Field order follows what you pass, so this puts shortname first. +py-moodle courses list --output json --fields shortname,id +``` + +Notes: + +- `--fields` only affects machine-readable output; it is ignored for the + default `--output table`. +- An unknown field name is a hard error (non-zero exit, the offending field + and the available fields are printed to stderr), so a typo fails loudly + instead of silently returning empty columns. +- Passing an empty value (`--fields ""`) is treated as "no filtering", + identical to omitting the flag. + ## Automate `py-moodle` in scripts and CI jobs Combine `--quiet`, `--no-color`, and `--output csv`/`--output json` to keep diff --git a/docs/roadmap-plan.md b/docs/roadmap-plan.md index 891d7af..6d473c7 100644 --- a/docs/roadmap-plan.md +++ b/docs/roadmap-plan.md @@ -56,6 +56,49 @@ maintainability without forcing broad refactors. The sequence below starts with the smallest changes that reduce risk and unlock later work. +### Progress update (2026-07-08) + +The phased plan below is the original forward-looking design and is kept intact +for reference. This section records what has actually landed so far, so the +plan can be read against reality. + +**Done:** + +- **Phase A** — Subtask 1 (this roadmap document), Subtask 2 (CI matrix now + covers Moodle 4.5.5 / 5.0.1 / 5.1.5 across Python 3.9–3.13), Subtask 3 + (test-layer & shared-fixture docs, #67/#69), Subtask 25 (HTML-fixture + regression scaffolding, #68/#70), Subtask 10 (task-oriented recipes, + including the `--fields` recipe). +- **Phase B** — Subtask 4 (`--output table|json|yaml`, plus `csv`, #25/#55), + Subtask 5 (`--fields` machine-readable field selection, #66/#71). +- **Phase C** — Subtask 6 (exception wording + `troubleshooting.md`, first + pass, #23), Subtask 7 (redacted `--debug` HTTP tracing, in `auth.py` and the + centralized `http.py` layer), Subtask 8 (centralized timeout policy in + `config.py`, #22/#39), Subtask 9 (bounded, backoff retry for idempotent + GET-style requests only — mutations are never auto-retried, #39). +- **Phase D** — Subtask 12/13 (typed `Course`, `CourseSection`, + `CourseModule`, `User`, `UploadResult` dataclasses in `models.py`), plus the + `MoodleClient` facade, `doctor`, and `ensure_course`/dry-run work + (#37–#57, #62, #64). +- **Phase G** — Subtask 15 (`--dry-run` for mutating commands), + partial Subtask 16 (`--force` / `ConfirmationRequired` on ensure paths). +- **Reliability/infra (not a numbered subtask)** — de-flaked the Docker-backed + integration suite (cross-worker course-creation lock #62; AJAX fallback on + webservice context errors #64; post-boot login warmup #74; ephemeral CI + containers + boot diagnostics #73) and fixed the Moodle 5.1 image boot + (moosh `public/` dir, erseco/alpine-moodle#149). + +**Partially done / next up:** + +- **Phase B** — Subtask 34 (CLI help/option consistency), 35 (exit codes & + batch summaries), 36 (shell completion). +- **Phase D** — Subtask 14 (ship `py.typed` now that the typed surface is + real), 17–20 (extend the ensure-style API beyond `ensure_course`: + `ensure_section`, `ensure_label`, `ensure_resource`, `ensure_folder`, + `create_or_update_course`). +- **Phase E/F** — compatibility-flow audit, hybrid backend selection, and the + module registration system remain largely greenfield. + ### Phase A: Lock in the baseline and smooth contributor workflows - **Subtask 1: Publish the technical baseline and roadmap plan.** diff --git a/src/py_moodle/http.py b/src/py_moodle/http.py index 5078976..f425e54 100644 --- a/src/py_moodle/http.py +++ b/src/py_moodle/http.py @@ -28,6 +28,7 @@ from __future__ import annotations import json +import logging import time from typing import Any, Dict, List, Mapping, Optional from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit @@ -66,6 +67,14 @@ #: Base backoff delay, in seconds, multiplied by the attempt number. _RETRY_BACKOFF_BASE_SECONDS = 0.01 +#: Logger for redacted HTTP-flow tracing. It is a child of the shared +#: ``py_moodle`` logger, so the CLI ``--debug`` flag (which raises that +#: logger to ``DEBUG``) surfaces these traces on stderr, and library users +#: can enable them with ``logging.getLogger("py_moodle").setLevel(DEBUG)``. +#: Only the request method, the *redacted* URL, and the response status are +#: ever logged -- never params, form/JSON bodies, headers, or response text. +_logger = logging.getLogger("py_moodle.http") + class MoodleHttpError(Exception): """Base exception for HTTP-layer failures talking to Moodle. @@ -264,12 +273,32 @@ def _send_request( request_kwargs.update(kwargs) secrets = _collect_secrets(url=url, params=kwargs.get("params"), headers=headers) + redacted_url = _redact_url(url) + method_label = method.upper() attempts = _RETRY_ATTEMPTS if retryable else 1 last_error_kind: Optional[str] = None for attempt in range(1, attempts + 1): + if attempt == 1: + _logger.debug("HTTP %s %s", method_label, redacted_url) + else: + _logger.debug( + "HTTP %s %s (retry %d/%d after %s error)", + method_label, + redacted_url, + attempt, + attempts, + last_error_kind, + ) try: - return call(url, **request_kwargs) + response = call(url, **request_kwargs) + _logger.debug( + "HTTP %s %s -> %s", + method_label, + redacted_url, + getattr(response, "status_code", "?"), + ) + return response except requests.exceptions.Timeout: last_error_kind = "timeout" except requests.exceptions.ConnectionError: @@ -277,7 +306,13 @@ def _send_request( if attempt < attempts: time.sleep(_RETRY_BACKOFF_BASE_SECONDS * attempt) - redacted_url = _redact_url(url) + _logger.debug( + "HTTP %s %s failed after %d attempt(s): %s error", + method_label, + redacted_url, + attempts, + last_error_kind, + ) message = _redact_text( f"Request to {redacted_url} failed after {attempts} attempt(s) " f"due to a {last_error_kind} error.", diff --git a/tests/unit/test_http.py b/tests/unit/test_http.py index 005e059..5150ceb 100644 --- a/tests/unit/test_http.py +++ b/tests/unit/test_http.py @@ -1,6 +1,7 @@ """Unit tests for the centralized HTTP layer in ``py_moodle.http``.""" import json +import logging import pytest import requests @@ -409,3 +410,52 @@ def test_request_html_get_uses_scrape_timeout_by_default(): request_html_get(session, "https://moodle.example.test/course/view.php?id=1") assert session.get_calls[0]["kwargs"]["timeout"] == DEFAULT_SCRAPE_TIMEOUT + + +# --------------------------------------------------------------------------- +# Redacted HTTP debug tracing +# --------------------------------------------------------------------------- + + +def test_send_request_emits_debug_trace_with_status(caplog): + """A request logs a method/URL trace and the response status at DEBUG.""" + session = StubSession(get_result=StubResponse(status_code=200, text="")) + + with caplog.at_level(logging.DEBUG, logger="py_moodle.http"): + request_html_get(session, "https://moodle.example.test/course/view.php?id=1") + + messages = [r.getMessage() for r in caplog.records if r.name == "py_moodle.http"] + assert any("HTTP GET" in m for m in messages) + assert any("-> 200" in m for m in messages) + + +def test_send_request_debug_trace_redacts_url_secrets(caplog): + """URL query secrets are redacted from the debug trace, never logged raw.""" + session = StubSession(get_result=StubResponse(status_code=200, text="")) + url = ( + "https://moodle.example.test/course/view.php" + f"?id=1&sesskey={FAKE_SESSKEY}&wstoken={FAKE_TOKEN}" + ) + + with caplog.at_level(logging.DEBUG, logger="py_moodle.http"): + request_html_get(session, url) + + messages = [r.getMessage() for r in caplog.records if r.name == "py_moodle.http"] + assert messages, "expected at least one py_moodle.http debug record" + joined = "\n".join(messages) + assert FAKE_SESSKEY not in joined + assert FAKE_TOKEN not in joined + # The redaction marker survives URL-encoding as its bare word (the + # asterisks are percent-encoded by urlencode), which is enough to prove + # the secret values were replaced rather than logged verbatim. + assert "REDACTED" in joined + + +def test_send_request_no_debug_trace_when_level_above_debug(caplog): + """No trace is emitted when the logger is above DEBUG (opt-in via --debug).""" + session = StubSession(get_result=StubResponse(status_code=200, text="")) + + with caplog.at_level(logging.INFO, logger="py_moodle.http"): + request_html_get(session, "https://moodle.example.test/course/view.php?id=1") + + assert [r for r in caplog.records if r.name == "py_moodle.http"] == []