Repository navigation
Conversation
added 4 commits
September 18, 2026 12:47
Three steps of the refactor plan had to be dropped half way because a name that looked private was in fact taught by the README or the docs (_list_async and friends, paginate_async, _meta as a dict). Nothing in the suite would have caught a break there. tools/differential/snapshot_public_api.py collects every `from lightapi... import ...` found in README.md, docs/ and examples/, keeps the ones that really import in the running version, and records the signatures of those classes and functions, plus the underscore helpers the docs tell users to call. tests/data/public_api.json is that snapshot taken from lightapi 0.1.29 installed from PyPI: 52 imports and 71 signatures. tests/test_public_api.py checks the working tree against it. A name may gain an optional parameter. It may not disappear, lose a parameter, reorder one, or start requiring one. The four additions made by the refactor (build_app(debug=), JWTAuthentication(extra_claims=), and page_size= on both paginators) pass because they are optional. Checked that it fails when it should: making build_app's debug required and dropping prev_cursor from CursorPaginator.wrap turns two tests red. Tests: 477 passed (354 + 123), 1 xfailed, 2 xpassed.
A refactor is supposed to change nothing a client can see, and the unit tests only check what someone thought of checking. tools/differential runs one fixed script of requests against whichever lightapi is installed and saves, for every step, the status, a few headers and the raw response text. compare.py then diffs two recordings. Raw text, not parsed JSON, because the key order of a response is part of what clients see. The script covers CRUD and optimistic locking on an in-memory and on a pooled engine, validation errors, 404/405/409, HEAD and OPTIONS, the four Serializer forms, field/search/ordering filters, page and cursor pagination, JWT and Basic auth with every permission shape, the login routes (JSON, Basic header, malformed, rate limited, custom auth_path), sync and async middleware, both CORS paths, full and partial reflection, a YAML app plus twelve invalid YAML documents, the same CRUD on an async engine with async querysets and verb overrides, background tasks, the response cache through a fake Redis, and the CREATE TABLE text of every table for SQLite and PostgreSQL. Only timestamps, JWTs, the rate-limit reset time and the order of the Allow header are normalised; the last two change with the process, not with the code. Recording the same version twice gives 412 identical steps, which is what makes a difference between two versions mean something. compare.py lists the differences the CHANGELOG declares and exits non-zero on any other. record.py refuses to run against the working tree, so it always tests an installed wheel or release, the thing users get.
While checking a fix, two comparisons in a row reported the old behaviour although the rebuilt wheel on disk had the new code. `uv run --with <local wheel>` had reused the environment it built for the previous wheel: a rebuild keeps the file name and the version, and --refresh-package did not change that. record.py now tells people to install each version into a venv of its own (with --no-cache --reinstall for a local wheel), and stores a fingerprint of the installed source in the recording. compare.py prints both fingerprints, so a B that did not change after a rebuild is easy to spot. Verified run: 0.1.29 against itself, 412 identical steps; 0.1.29 against the wheel of this branch, 406 identical, 6 different and all declared in the CHANGELOG, 0 undeclared.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 37435065 | Triggered | Base64 Basic Authentication | 8642855 | tools/differential/scenarios_sync.py | View secret |
| 37435064 | Triggered | Generic Password | 8642855 | tools/differential/scenarios_sync.py | View secret |
| 37435064 | Triggered | Generic Password | 8642855 | tools/differential/diff_validators.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
This was referenced Sep 18, 2026
Closed
added 5 commits
September 18, 2026 13:53
The plan's Level 3 (T3-01 to T3-04, T3-08, T3-09) had never been run: all 354+ existing tests use SQLite and a mocked Redis. tests/integration/ adds 19 tests, skipped unless the plan's environment variables are set (LIGHTAPI_TEST_PG_SYNC, LIGHTAPI_TEST_PG_ASYNC, LIGHTAPI_REDIS_URL), so the normal suite is unaffected: 502 passed, 19 skipped, 1 xfailed, 2 xpassed, same as before this commit plus the skips. - test_postgres_crud.py: sync CRUD, optimistic locking, 404/409/422 against a real Postgres engine with a real connection pool (T3-01). - test_postgres_async.py: the same through asyncpg and AsyncSession.run_sync, plus Decimal/UUID/datetime/bool round-trips (T3-02). - test_postgres_reflection.py: full and partial reflection of a table with BIGINT, NUMERIC, DOUBLE PRECISION, UUID, JSONB, INET, ARRAY and more; confirms every reflected column, nullable or not, is a required key in the create schema, JSONB/INET/ARRAY fall back to Any with a warning and can be sent as null (T3-03). - test_postgres_concurrency.py: 20 real concurrent writers against one row, sync and async; exactly 1 succeeds and 19 get 409 (T3-04). - test_redis_cache.py: Meta.cache against a real Redis, sync and async, hit/miss/invalidation, and the RuntimeWarning-but-still-serves path when Redis is unreachable (T3-08, T3-09). Found and worked around a pre-existing bug (not a regression, identical on 0.1.29): LightApi._create_tables() detects a running event loop and creates tables via asyncio.run() in a worker thread; for a loop-bound async driver (asyncpg, and aiosqlite's shared in-memory connection) the next request can be handed a connection tied to that now-dead loop and fail. Documented in test_postgres_async.py's module docstring and pinned by test_the_pool_poisoning_bug_is_reproducible; every other test here builds the app before any event loop is running, or creates its tables directly on the real loop first. Also fixed along the way, all self-inflicted test bugs rather than framework issues: reusing one RestEndpoint class across an isolated and a non-isolated app (or two isolated apps) leaves _model_class set from the first mapping, so the second app silently keeps operating on the first app's table -- each engine/isolation combination now gets its own class. Postgres tables persist across local runs of the same disposable container, so CRUD/concurrency fixtures truncate before each test. pytest.ini registers the "integration" marker (this file, not pyproject.toml, is the one pytest actually reads here). Tests: 502 passed, 19 skipped (without the env vars), 1 xfailed, 2 xpassed. All 19 pass against real Postgres 16 and Redis 7 containers, repeatably across multiple runs against the same persistent container.
- test_cli_serve.py: `lightapi serve` and `python -m lightapi` as real subprocesses against docker/lightapi.example.yaml, plus the three error paths (no LIGHTAPI_CONFIG, bad LIGHTAPI_PORT, unknown subcommand) all exiting non-zero with a message naming the problem (T4-01, T4-02, T4-03). - test_server_lifecycle.py: build_app(debug=True) shows a traceback, debug=False does not (T4-05); cors_origins wraps the whole app and handles preflight, lightapi.core.CORSMiddleware only reaches requests Starlette already routes to the endpoint (OPTIONS is not one, since the Route's methods never include it -- confirmed identical on 0.1.29, not a regression) (T4-06); the login rate limiter returns 429 with Retry-After after the configured limit, and the 1000/min default leaves normal use alone (T4-07). Also ran, not as an automated test because it needs two full Python environments and a timing comparison rather than a pass/fail assertion (T4-04, the strongest claim of the whole series): a real uvicorn server, Postgres pg_sleep(2) behind a sync GET, and /healthz timed 200ms later on a second connection. Two runs each: 0.1.29 answered healthz in 1.87s and 1.88s (blocked, nearly the full 2s); this branch in 0.378s and 0.007s (free). The scripts are in the session scratchpad, not committed here. Tests: 513 passed, 19 skipped (integration, needs Postgres/Redis), 1 xfailed, 2 xpassed.
Three cases from the test plan were marked "Parcial" because the
existing test only covered part of what the case asks for.
- T7-03: test_sync_verb_override.py only covered a sync def get. Added
post/put/patch/delete overrides (each stamps a header so the test can
tell the override ran, not the built-in method) and a case for the
detail route, since a sync def get also has to intercept GET /x/{id}.
- T7-04: test_login_backend_subclass.py only covered JWTAuthentication
subclasses. Added a BasicAuthentication subclass: /auth/login uses its
validate_credentials, wrong credentials still 401, and the subclass
still guards the endpoint with a real Basic header.
- T7-07: test_crud.py's PATCH tests never included a nullable field, so
the three null-handling rules in _update_values had no test: null
clears a nullable column, null on a non-nullable column is ignored
(not an error), and an omitted field leaves the column as it was.
Tests: 524 passed, 19 skipped, 1 xfailed, 2 xpassed.
Extracts the Quick Start python block from README.md and executes it
verbatim (minus swapping the on-disk sqlite path for a tmp one and
disabling the __main__ app.run() call) so a doc edit that breaks the
example fails this test instead of going unnoticed.
The example must use a real sqlite FILE, matching the README's own
sqlite:///books.db: create_engine("sqlite://") (in-memory) defaults to
SingletonThreadPool, a separate empty database per thread, and
TestClient may run the request on a different thread than the one
that just built the app and created the table.
Claude-Session: https://claude.ai/code/session_01Kmta8V3goGySJyWVEkgweD
…ctor (T6-03) Parses every ```python fence in the ten changed doc pages (ast.parse), then executes the handful that are complete, runnable scripts as real subprocesses: the async/middleware pytest-asyncio fixture pattern, and the authentication "Complete Example". The doc's own async testing pattern (building the app inside an already running loop, exactly as pytest_asyncio.fixture does) deterministically reproduces the pre-existing table-creation bug already tracked in tests/integration/test_postgres_async.py (B19): LightApi._create_tables() creates tables via asyncio.run() in a worker thread when a loop is already running, which for aiosqlite/asyncpg poisons the engine's connection pool with a connection bound to that dead thread's loop. Every later async ORM call then fails with MissingGreenlet, and the create_all exception itself is swallowed to a warning log. Pinned as a reproduction rather than worked around, since this is the first place it's shown to hit the docs' own recommended pattern, not just Postgres. Claude-Session: https://claude.ai/code/session_01Kmta8V3goGySJyWVEkgweD
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #42. Last in the series: it adds no feature, it checks the other five PRs.
What
Two test tools, and what they found.
tests/test_public_api.py: the API that the README, docs and examples teach must keep working.tools/differential/snapshot_public_api.pycollects everyfrom lightapi... import ...in README.md, docs/ and examples/, keeps the ones that really import, and records the signatures.tests/data/public_api.jsonis that snapshot taken from lightapi 0.1.29 installed from PyPI: 52 imports, 71 signatures. A name may gain an optional parameter; it may not disappear, lose a parameter, reorder one, or start requiring one. It fails when I makebuild_app'sdebugrequired or dropprev_cursorfromCursorPaginator.wrap.tools/differential/: one fixed script of 412 steps run against whichever lightapi is installed, saving status, a few headers and the raw response text of every step, plus theCREATE TABLEtext of all 45 tables for SQLite and PostgreSQL.compare.pydiffs two recordings and exits non-zero on any difference the CHANGELOG does not declare. It covers CRUD and optimistic locking (in-memory and pooled engine), validation errors, 404/405/409, the four Serializer forms, filters, page and cursor pagination, JWT and Basic auth with every permission shape, the login routes, sync and async middleware, both CORS paths, full and partial reflection, a YAML app and twelve invalid YAML documents, the same CRUD on an async engine, async querysets and verb overrides, background tasks, and the response cache through a fake Redis.What it found
A regression in #40 that no test had caught. Reflected float columns came back as a JSON number (
0.5) where 0.1.29 returns a JSON string ("0.5"). In SQLAlchemyFloat,DoubleandREALare subclasses ofNumeric, and the old type chain testedNumericfirst, so a reflected float has always validated asDecimal.column.type.python_typesaysfloat. I had written in #40 that the two agreed; I had comparedpython_typewith what I expected the old chain to return, not with what it returned.Running the real old function next to the new one over 43 column types showed 8 differences: every float type, and
Uuid(as_uuid=False). The fix is commit 67b8d78 on the #40 branch, merged up through #41 and #42 into this one, withtests/test_reflected_annotations.pypinning 25 column types (8 fail without the fix). The same comparison now shows 0 differences.Whether a reflected float should be a JSON number is a fair question. It changes the wire format, so it needs a decision and a changelog entry, not a refactor.
A trap in the tooling. Two comparisons in a row reported the old behaviour after the fix.
uv run --with <local wheel>had reused the environment built for the previous wheel, because a rebuild keeps the file name and version.record.pynow says to install each version into its own venv with--no-cache --reinstall, and stores a fingerprint of the installed source thatcompare.pyprints.Result
Installed and verified by fingerprint:
def get/postoverrides are served (3), login uses the subclass backend (1),Meta.cacheworks on an async engine (2).Also run for this PR, all on my machine:
httpx<0.28, because starlette 0.37's TestClient does not work with newer httpx; that is a test dependency, not a package one.twine checkpasses, and it installs and serves a sync app on Python 3.10 with no async driver present.Not green, and not from these PRs
tests/test_filtering.py,tests/test_http_methods.py). Plaindevfails on the same seeds with the same counts, so the refactor added no order dependence, but the suite has one.AsyncEnginealone does not switch the app to async. The README calls it a one-line swap, but withoutmode="async"or anasync defmethod the app stays sync, table creation fails with a swallowed warning, and every request is a 500. Same in 0.1.29; the harness records it so it stays visible.Checks
pytest: 502 passed, 1 xfailed, 2 xpassed (379 from fix: lazy Redis client; deprecate LightApi.from_dict #42 after the fix was merged up, plus 123 here). Green on Python 3.10 and 3.12 too.lightapi/: 88% (2,099 statements, 244 missed).ruff check,ruff format --check,mypy: clean.Not done: Postgres, a real Redis, a real uvicorn server, Docker, Helm, the docs snippets. Those are levels 3 to 8 of the test plan.