chore(api): stop injecting SessionId into merged spec - #102
chore(api): stop injecting SessionId into merged spec#102hotdata-automation[bot] wants to merge 1 commit into
Conversation
| }; | ||
| req_builder = req_builder.header("X-Session-Id", value); | ||
| }; | ||
| if let Some(token) = configuration.resolve_bearer_token().await { |
There was a problem hiding this comment.
Dropping the X-Session-Id block here (and in results_api::get_result / list_results) leaves the SDK internally inconsistent, because the hand-written mirrors of these same requests still send it:
src/query.rs:618(send_query, the pathClient::queryactually uses) → sendsX-Session-Idsrc/client.rs:468(Client::submit_query) → sendsX-Session-Idsrc/arrow.rs:305(apply_apikey_headers, used byget_result_arrow/stream_result_arrow) → sendsX-Session-Id
After this change there is no X-Session-Id emission left anywhere in src/apis, so a single Client::query() call now submits with the session header (send_query) and then polls GET /v1/results/{id} without it (wait_for_result → generated get_result). Same for JSON vs Arrow result fetches: the Arrow path is session-scoped, the generated path is not.
This also silently narrows public API surface: ClientBuilder::session_id (src/client.rs:126) and the HOTDATA_SESSION_ID env var still install the key into configuration.api_keys and their docs still say "Installed as the X-Session-Id header when present" — which is now only true for the hand-written paths. If the backend still scopes results by session, a session-scoped client's get_result / list_results calls are no longer scoped.
Please resolve one way or the other before merge:
- If session auth is genuinely gone from the API — finish the removal in the same PR: drop
ClientBuilder::session_id,SESSION_ID_HEADER, theHOTDATA_SESSION_IDlookup, theSESSION_ID_HEADERentries insrc/query.rs/src/arrow.rs/src/client.rs::submit_query, the now-wrongarrow.rstests and doc comments that say "mirroring the generatedget_result" (src/arrow.rs:301,src/arrow.rs:496,src/query.rs:639), and theX-Session-Idrationale in.openapi-generator-templates/README.md:7. Record it in the changelog as Breaking sincesession_id()is public API. - If session scoping is still enforced server-side — the spec change is wrong and this regen should not ship.
|
|
||
| ### Changed | ||
|
|
||
| - chore(api): stop injecting SessionId into merged spec |
There was a problem hiding this comment.
nit: (not blocking) this entry is the raw commit subject and describes the spec pipeline ("merged spec"), not what changes for a user of this crate. Every other entry in this file is written from the caller's point of view. Something like "query, get_result, and list_results no longer send the X-Session-Id header" would tell a reader whether they are affected.
There was a problem hiding this comment.
Review
Blocking Issues
src/apis/query_api.rs:57,src/apis/results_api.rs:76,src/apis/results_api.rs:150— removing theX-Session-Idemission leaves no session-header emission anywhere insrc/apis, while the hand-written mirrors of the same requests still send it:src/query.rs:618(send_query),src/client.rs:468(submit_query),src/arrow.rs:305(Arrow result fetch). Net effect: oneClient::query()call now submits with the session header and then pollsGET /v1/results/{id}without it, and JSON vs Arrow result fetches disagree on scoping.src/client.rs:126—ClientBuilder::session_idand theHOTDATA_SESSION_IDenv var are public API. They still install the key and still document "Installed as theX-Session-Idheader when present", which is now only true on the hand-written paths. If the backend still scopes results by session, session-scopedget_result/list_resultscalls are silently no longer scoped.- Stale statements left behind:
src/arrow.rs:301andsrc/query.rs:639say the helpers mirror the generated ops'isKeyInHeaderblocks; the test atsrc/arrow.rs:496asserts behavior described as "matching the generatedget_result". Neither holds after this change..openapi-generator-templates/README.md:7still citesX-Session-Idas the reason for the template override.
Action Required
Pick one and land it in this PR:
- Session auth is gone from the API → finish the removal: drop
ClientBuilder::session_id,SESSION_ID_HEADER, theHOTDATA_SESSION_IDlookup, the session entries insrc/query.rs/src/arrow.rs/Client::submit_query, and the now-wrong tests and doc comments listed above. Record it as a Breaking changelog entry, sincesession_id()is public. - Session scoping is still enforced server-side → the upstream spec change is wrong and this regen should not ship; fix it at the source (the linked www.hotdata.dev PR 283).
Also see the non-blocking changelog-wording comment.
Auto-generated from the updated HotData OpenAPI spec.
Source: https://github.com/hotdata-dev/www.hotdata.dev/pull/283