chore(uploads): document 16 GiB upload size limit - #191
chore(uploads): document 16 GiB upload size limit#191hotdata-automation[bot] wants to merge 1 commit into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| ### Changed | ||
|
|
||
| - chore(uploads): document 16 GiB upload size limit |
There was a problem hiding this comment.
The changelog records only the upload-limit doc change, but this regeneration also removes and hardens public API surface. Add the breaking entries under ### Removed and ### Changed, matching the existing convention at CHANGELOG.md:10-20 and the 0.9.0 entry for partition_by / sorted_by.
Four breaking items are undocumented:
ConnectionsApi.purge_connection_cacheis removed.ConnectionsApi.purge_table_cacheis removed.CreateConnectionRequest.skip_discoveryis removed.TableInfo.constant_per_keyis added as a required field (hotdata/models/table_info.py, no default).
Failure scenario: a user on 0.10.0 upgrades and calls connections_api.purge_connection_cache(cid). The call raises AttributeError at runtime. A user who builds a TableInfo(...) fixture without constant_per_key gets a pydantic ValidationError. Neither break is mentioned anywhere in the changelog.
scripts/check-release.py only compares the changelog against a version bump, and pyproject.toml still reads 0.10.0, so CI does not catch this gap.
| """Get result | ||
|
|
||
| Retrieve a persisted query result by ID. The response format for the `ready` state is selected by `Accept` header or `?format=` query param; non-ready states use the same status codes and JSON body shape regardless of format. | Result status | Status × body | |-----------------------|------------------------------------------------------------------------------| | `ready` + JSON | 200 `application/json` — `GetResultResponse` with `columns`, `rows`, etc. | | `ready` + Arrow | 200 `application/vnd.apache.arrow.stream` — schema, RecordBatches, EOS | | `ready` + CSV | 200 `text/csv; charset=utf-8` — single header row, streamed batch-by-batch | | `ready` + Markdown | 200 `text/markdown; charset=utf-8` — GitHub-flavored pipe table, streamed | | `ready` + Parquet | 200 `application/vnd.apache.parquet` — raw parquet bytes (no conversion) | | `pending`/`processing`| 202 `application/json` `{status, result_id}` + `Retry-After` | | `failed` | 409 `application/json` `{status, result_id, error_message}` | | not found | 404 `application/json` (`ApiErrorResponse`) | `?format=` accepts `arrow`, `json`, `csv`, `md`, `parquet` and takes precedence over `Accept`. `markdown` is accepted as a runtime alias for `md`. Use `?offset=N&limit=M` to slice the result; `offset` defaults to 0 and `limit` is unbounded by default. Both must be non-negative; invalid values return 400. When a finite `limit` doesn't reach the end of the result, a `Link` header with `rel=\"next\"` points at the following page. `?offset`/`?limit` are ignored for `format=parquet` since that path returns the underlying file unchanged. Ready responses (Arrow, CSV, Markdown, JSON) carry `X-Total-Row-Count` (the full result row count, independent of offset/limit). Responses are streamed end-to-end, so a client can disconnect at any time and the server stops reading. IEEE special floats (`±Inf`, `NaN`) have no canonical JSON representation. For cross-format consistency the JSON, CSV, and Markdown paths emit them as `null` / empty cells, and JSON `nullable[]` is widened to match. The Arrow IPC and Parquet bodies are binary round-trip formats and preserve the raw IEEE values; callers cross-checking a result across CSV and Parquet should not byte-compare those slots. | ||
| Retrieve a persisted query result by ID. The response format for the `ready` state is selected by `Accept` header or `?format=` query param; non-ready states use the same status codes and JSON body shape regardless of format. | Result status | Status × body | |-----------------------|------------------------------------------------------------------------------| | `ready` + JSON | 200 `application/json` — `GetResultResponse` with `columns`, `rows`, etc. | | `ready` + Arrow | 200 `application/vnd.apache.arrow.stream` — schema, RecordBatches, EOS | | `ready` + CSV | 200 `text/csv; charset=utf-8` — single header row, streamed batch-by-batch | | `ready` + Markdown | 200 `text/markdown; charset=utf-8` — GitHub-flavored pipe table, streamed | | `ready` + Parquet | 200 `application/vnd.apache.parquet` — raw parquet bytes (no conversion) | | `pending`/`processing`| 202 `application/json` `{status, result_id}` + `Retry-After` | | `failed` | 409 `application/json` `{status, result_id, error_message}` | | not found | 404 `application/json` (`ApiErrorResponse`) | `?format=` accepts `arrow`, `json`, `csv`, `md`, `parquet` and takes precedence over `Accept`. `markdown` is accepted as a runtime alias for `md`. Use `?offset=N&limit=M` to slice the result; `offset` defaults to 0 and `limit` is unbounded by default. Both must be non-negative; invalid values return 400. When a finite `limit` doesn't reach the end of the result, a `Link` header with `rel=\"next\"` points at the following page. `?offset`/`?limit` are ignored for `format=parquet` since that path returns the underlying file unchanged. JSON is the only format with a size limit. Building a JSON body requires holding the whole slice in memory at once, so a request whose JSON body would exceed the instance's per-fetch memory budget is refused with 413 and the error code `RESULT_TOO_LARGE`, and a request that would push the total across concurrent JSON fetches over that budget is refused with 429. If the result's size cannot be determined at all, the fetch is refused with 503 rather than served unchecked. None of the three returns partial data. To read a result of any size, request it as `?format=arrow`, `csv`, `md`, or `parquet` — those stream and are never size-limited. Paging with `offset`/`limit` is not an alternative: the rows come back in no guaranteed order, so pages are not a reliable way to read one result in parts. Ready responses (Arrow, CSV, Markdown, JSON) carry `X-Total-Row-Count` (the full result row count, independent of offset/limit); the JSON body repeats it as `total_row_count`, so a client that cannot read response headers can still compare it against `row_count` to tell a complete result from a windowed one. Responses are streamed end-to-end, so a client can disconnect at any time and the server stops reading. IEEE special floats (`±Inf`, `NaN`) have no canonical JSON representation. For cross-format consistency the JSON, CSV, and Markdown paths emit them as `null` / empty cells, and JSON `nullable[]` is widened to match. The Arrow IPC and Parquet bodies are binary round-trip formats and preserve the raw IEEE values; callers cross-checking a result across CSV and Parquet should not byte-compare those slots. |
There was a problem hiding this comment.
This regenerated docstring states that offset/limit paging is not a reliable way to read one result in parts, because rows come back in no guaranteed order. hotdata/query.py does exactly that.
QueryClient._fetch_all_rows (hotdata/query.py:531-551) reassembles a truncated result by looping results_api.get_result(result_id, offset=offset, limit=page_size, format=ResultsFormatQuery.JSON). That loop assumes a stable row order across requests.
Failure scenario: query(sql, auto_follow=True) returns a result whose total exceeds poll.page_size. Page 1 and page 2 are served with different row orders. The returned rows then holds duplicated rows and omits others, and the caller sees a silently wrong result set — total_row_count still matches, so nothing signals the corruption.
Fix: switch the auto-follow path to the streaming Arrow reader already in this repo, ArrowResultsApi.stream_result_arrow (hotdata/arrow.py:122). The same docstring states the streaming formats are never size-limited, which also removes the newly added 413 / 429 / 503 JSON refusals from the auto-follow path.
If the server does guarantee a stable order across paged reads of one result, correct the spec text instead, and this comment does not apply.
|
|
||
| @validate_call | ||
| def purge_table_cache( | ||
| def set_managed_table_constant_per_key( |
There was a problem hiding this comment.
nit: purge_connection_cache is gone, but tests/integration/test_connections_read.py:32 still calls it, and the module docstring at line 3 still advertises "cache purge". The test carries @pytest.mark.skip, so CI stays green and the stale call is invisible until someone re-enables the test and hits an AttributeError. Drop the call and the docstring clause in the same PR that removes the method. (not blocking)
There was a problem hiding this comment.
Review
The full diff was omitted from the review prompt. gh pr diff 191 was fetched and read in full (5374 lines). Files read from the checkout for surrounding context: CHANGELOG.md, pyproject.toml, scripts/check-release.py, hotdata/query.py, hotdata/uploads.py, hotdata/arrow.py, tests/integration/test_connections_read.py, .github/workflows/check-release.yml, .github/workflows/integration-tests.yml.
The generated code itself is internally consistent: models, __init__ exports, .openapi-generator/FILES, docs, and generated tests all agree. The problems sit at the edges of the regeneration.
Blocking Issues
-
CHANGELOG.md:24— the changelog records only the upload-limit doc change. This regeneration also removesConnectionsApi.purge_connection_cache,ConnectionsApi.purge_table_cache, andCreateConnectionRequest.skip_discovery, and makesTableInfo.constant_per_keya required field. None of the four is documented. The repo convention for exactly this is atCHANGELOG.md:10-20and in the 0.9.0 entry.scripts/check-release.pyonly fires on a version bump andpyproject.tomlstill reads0.10.0, so CI does not catch the gap. -
hotdata/api/results_api.py:67— the newget_resultdocstring states thatoffset/limitpaging is not a reliable way to read one result in parts, because rows come back in no guaranteed order.QueryClient._fetch_all_rows(hotdata/query.py:531-551) reassembles a truncated result by exactly that paging loop. Under the newly documented contract,query(sql, auto_follow=True)can return duplicated and missing rows with no signal to the caller.
Action Required
- Add
Removedentries marked**Breaking:**for the two removedConnectionsApimethods and forCreateConnectionRequest.skip_discovery, and aChangedentry forTableInfo.constant_per_keybecoming required. - Either move the auto-follow path in
hotdata/query.pyontoArrowResultsApi.stream_result_arrow(hotdata/arrow.py:122), which the same docstring says is never size-limited, or correct the spec text if paged reads of one result do preserve order.
Notes on CI
No check is reported as passing in the snapshot taken at review start. Nothing here was run locally.
One non-blocking comment is left inline on hotdata/api/connections_api.py.
Auto-generated from the updated HotData OpenAPI spec.
Source: https://github.com/hotdata-dev/www/pull/425