Skip to content

Add FastAPI testing-environment status API (maintainer-gated) - #13278

Open
RayBB wants to merge 3 commits into
masterfrom
testing-env/status-api
Open

Add FastAPI testing-environment status API (maintainer-gated)#13278
RayBB wants to merge 3 commits into
masterfrom
testing-env/status-api

Conversation

@RayBB

@RayBB RayBB commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Exposes the data behind the /status deploy table ("Testing Environment") as a JSON API, so the status page can later be rebuilt as an interactive component.

New FastAPI endpoint

https://testing.openlibrary.org/_fast/status/testing.json

GET /status/testing.json (maintainer-gated) returns the testing environment state:

{
  "last_deploy_at": "2026-08-05T18:00:00+00:00",
  "has_pending": true,
  "prs": [{
    "pr": 13269, "title": "...", "commit": "<full sha>",
    "active": true, "pending_active": null,
    "added_at": "...", "added_by": "openlibrary",
    "author": "...", "author_avatar": "...",
    "assignee": "...", "assignee_avatar": "...",
    "pull_latest_sha": "",
    "head_sha": "abc1234", "drift": 0, "merged": false, "is_new": true
  }]
}
  • Lives in openlibrary/fastapi/status.py (FastAPI land, not web.py), with room for more status endpoints later.
  • Gated to maintainers/admins via a new require_maintainer auth dependency — same access level as the existing /status page (/usergroup/maintainers or /usergroup/admin).
  • Returns 404 if no testing state exists.

How it works

  • get_testing_status() extracted in openlibrary/plugins/openlibrary/status.py as a single source of truth, now shared by the legacy /status page and the new endpoint so they can't drift apart.
  • User.is_maintainer() added to the User model; the web.py _is_maintainer() delegates to it.
  • _testing-prs.json is auto-created (empty) at app startup if missing, so the page/API work from first boot without a manual file. It is gitignored and never overwritten.

Test plan

  • 404 FastAPI tests pass, plus new tests for the endpoint, the helper, require_maintainer, and the state-file bootstrap.
  • Live smoke test against a local dev environment:
    • no cookie → 401
    • logged in as openlibrary (admin) → 200 with full payload incl. GitHub drift (drift: 4, merged: true)
  • pre-commit clean on all touched files.

Notes

  • The endpoint lives in FastAPI; include_in_schema is gated to local dev (LOCAL_DEV), matching the internal router pattern.
  • Session-cookie auth is used, so the endpoint works with the same login as the web page.

lokesh
lokesh previously requested changes Aug 6, 2026

@lokesh lokesh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Claude review below overstates the issues and confused itself.

Claude review:
Blocking

  • openlibrary/plugins/openlibrary/status.py:614: Creating the state file in setup() removes the only thing gating this UI. Right now the absence of _testing-prs.json is what keeps show_testing false, and templates/status.html:51 hangs the entire deploy table, the "Add PR(s)" form, and the Deploy button off it. Once every server writes the file at boot, any maintainer loading /status on production gets that UI, and _trigger_rebuild() is one click away whenever config.jenkins_token is set. It also makes the documented 404 at openlibrary/fastapi/status.py:59 unreachable, so a client can't tell "not a testing environment" from "nothing pinned". Let's drop the startup write and gate on something explicit, a config flag or get_deployment_name() in openlibrary/core/env.py.

  • openlibrary/plugins/openlibrary/status.py:424: TESTING_STATE_FILE is CWD-relative, and nothing guarantees the ASGI process shares a working directory with the web.py process. compose.production.yaml doesn't bind-mount the repo into web or fast_web (staging and the override file do, which is why this looks fine locally), so each container ends up with its own copy. Writes happen in web, reads in fast_web, and /status/testing.json answers 200 {"prs": []} forever while the page shows the real list. That's the drift this PR is meant to remove. State that two processes share should live in the DB or memcache rather than a per-container file.

  • openlibrary/fastapi/status.py:58: A read endpoint shouldn't write. On a cache miss get_testing_status() falls through to _get_drift_info(), which mutates the PRs in place and calls _save_testing_state(). So a testing.json request that loaded state before a maintainer's /status/deploy can finish its GitHub refresh afterward and write the stale copy back, silently undoing the deploy. The same path makes up to two sequential GitHub calls per PR at a 5s timeout each with no overall budget, on a plain GET. Could the endpoint take a cache-only path that skips both the refresh and the save?

  • openlibrary/plugins/openlibrary/status.py:431: _ensure_testing_state_file() writes at plugin-load time in every process that loads the OL plugins, including standalone scripts and the pytest run. An OSError there propagates out of setup() and the process fails to boot. Previously that file was only written inside maintainer-authenticated POST handlers, so an unwritable CWD was harmless. The exists() check also won't repair a zero-byte file, and write_text truncates before writing, so a crash mid-write leaves json.loads("") raising a 500 on both /status and testing.json.

Has this been through a deploy shaped like production, with web and fast_web in separate containers? Most of the above only shows up there.

@RayBB

RayBB commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

I’m not sure that any of this review from Claude makes sense. All of this is gated behind off, so only maintainers can see it. The testing state file is something that already exists and we can see it working in the ASGI process. That’s not something that was added as part of this pull request. it be a file or live in a database? I don’t know. I’m building on top of Mac's pull request. the endpoint shouldn write I think writing to the thing when you get updated information is fine I think this is basically what we're already doing. And the last one, I mean, it basically seems fine. need to either have every user create this file or create it automatically in some way. This isn't something that almost anyone will be using locally, but if we want people to be able to test it and develop it, then we need the setup. And also, obviously, it has not been through a production deploy because it's a pull request that I just opened. So I don't know if, your AI can take into account how testing and production actually works. Maybe it would make some more useful feedback.

@RayBB
RayBB requested a review from lokesh August 7, 2026 00:05
@lokesh
lokesh dismissed their stale review August 7, 2026 02:40

Blockers were not validated by a human.

@lokesh

lokesh commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

@RayBB one question, with the addition of _ensure_testing_state_file, would the deploy UI now show up on the production status page for maintainers?

@RayBB

RayBB commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

That's a great question. I think we should probably as a check so that it doesn't. In fact this really only needs to run in local envs. I'll do that. Don't let it stop you from thinking on the ui though

@RayBB

RayBB commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

@lokesh fixed that issue

RayBB added 3 commits August 12, 2026 12:45
Expose the /status deploy table data (testing environment) via a new
FastAPI endpoint GET /status/testing.json, gated to maintainers/admins.

- Extract get_testing_status() as a shared source of truth used by both
  the legacy /status page and the new API
- Add User.is_maintainer() and require_maintainer/MaintainerDep, mirroring
  the existing librarian auth dependency
- Auto-create _testing-prs.json at startup (if missing) so the page and
  API work from first boot without a manual file
- Add tests for the helper, endpoint, and auth gating
The deploy UI must not appear on the production status page. Gate the
_ensure_testing_state_file() bootstrap behind get_ol_env().LOCAL_DEV so
the file is only auto-created for local development.
@RayBB
RayBB force-pushed the testing-env/status-api branch from cdda7c4 to e367a73 Compare August 12, 2026 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants