Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

Split connect/read timeouts and add an overall deadline to /authenticate

Aperta
#185 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
45/100
Tipo di issue
Funzionalità
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
fastapi, python

Direzione di ricerca

Start after #184 merges, then read _fetch_new_client_with_csrf_token in app/pesu.py and PESUAcademy.authenticate, along with the timeout guidance in CONTRIBUTING.md. Add the configured phase timeouts, request deadline handling, client-close coverage, and the README note; verify the listed timeout, cancellation, deadline, and within-deadline tests plus the live suite.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

enhancement pesuacademy

🔄 Updated for v5.0.0 (2026-10-08). A login is now at most two upstream calls (login and profile_fetch) on one client created in authenticate, with no CSRF fetch and no redirects followed (#241). The worst case is about 20 s rather than 30 s, and the background and the steps below are updated to match. The proposal itself is unchanged.

⚠️ PES University students only. This change sits in the login path and has to be tested end to end against PESU Academy with real PESU credentials (see "Tests that need credentials" in CONTRIBUTING.md). Run the whole suite with your credentials in .env and paste the summary line into your PR. Every run makes real logins to PESU Academy, so run it when you need it, not in a loop.

Summary

Every upstream call uses a single 10-second timeout for every phase, and there's no limit on a whole request. A login with profile: true makes two calls, so a caller can wait about 20 seconds before getting an answer. Many HTTP clients and proxies give up well before that. Use separate connect and read timeouts, and add one overall deadline per /authenticate request.

Background

  • httpx2.AsyncClient(timeout=UPSTREAM_TIMEOUT_SECONDS) in PESUAcademy.authenticate (app/pesu.py), with UPSTREAM_TIMEOUT_SECONDS = 10.0. A bare number means connect, read, write and pool are each 10 s.
  • A login makes a login POST and, when the profile is requested, a profile_fetch POST, both on the same client. Redirects are not followed.
  • A connect timeout should be short: if the TCP/TLS connection can't be made in a few seconds, it won't be. A read timeout needs to be longer, because PESU Academy can be slow to answer: in the 5.0.0 benchmarks the login took about 2.5 s on average and the profile call about 0.9 s.
  • pesu_auth_upstream_latency_seconds{operation} in /metrics has the real numbers. Ask a maintainer for production values if /metrics is token-protected.

Proposed change

  1. Per-phase timeouts: httpx2.Timeout(read=READ_TIMEOUT, connect=CONNECT_TIMEOUT) (for example connect 5 s, read 10 s), defined as module constants with a comment saying where the numbers came from (the latency metrics). Note that httpx2.Timeout(10.0, connect=5.0) sets connect to 5 s and everything else to 10 s.
  2. Overall deadline: wrap the work in PESUAcademy.authenticate in async with asyncio.timeout(REQUEST_DEADLINE): (for example 20 s).
    • When it fires, asyncio.timeout raises TimeoutError. Convert that to the 504 exception from #184, with the same message.
    • The client must still be closed; the existing finally handles this, but add a test.
    • Everything a login does happens inside authenticate (5.0.0 removed the background prefetch), so the deadline covers all of it.
  3. Document the timeouts in the README (a short "Timeouts" note under /authenticate) so callers can set their own client timeout above the deadline.

Depends on #184, which adds the 504 exception. Start after it merges, or coordinate with its assignee.

Tests

  • The client is built with the configured httpx2.Timeout (assert on the constructor arguments).
  • A request whose upstream calls together exceed the deadline returns 504, and the client is closed (patch the calls with asyncio.sleep and a small deadline).
  • A request within the deadline is unaffected.
  • Cancellation from outside still propagates as CancelledError, not 504.
  • The live suite passes.

Acceptance criteria

  • Connect and read timeouts are separate named constants with a documented rationale
  • No /authenticate request runs longer than the deadline (plus a small margin)
  • Deadline expiry is a 504 in the standard error body, documented in the README

How to contribute

  • Wait until you're assigned. Comment to ask for this issue, then wait for a maintainer to assign it to you before writing any code. PRs for unassigned issues are closed without review.
  • Read CONTRIBUTING.md first. It covers forking, setting up with uv, and running the tests.
  • Branch from dev in your fork, and open the PR against dev with a Conventional Commit title and Closes #185 in the description.
  • Bump the minor version in pyproject.toml and run uv lock (CI checks this). uv run pre-commit run --all-files must pass, and coverage must stay at 100%.
  • Update the README, app/docs/ and comments in the same PR as the behaviour they describe.
  • Never log, print, store or echo a password or token, including in tests, PR descriptions and screenshots.
Lingua principale
Python
Stelle
22
Fork
29
Merge medio
22h 1m
PR unite (30g)
20

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di pesu-dev/auth

Tutte le issue di pesu-dev/auth

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.