Skip to content

chore(api): stop injecting SessionId into merged spec - #102

Open
hotdata-automation[bot] wants to merge 1 commit into
mainfrom
openapi-update-30398808397
Open

chore(api): stop injecting SessionId into merged spec#102
hotdata-automation[bot] wants to merge 1 commit into
mainfrom
openapi-update-30398808397

Conversation

@hotdata-automation

Copy link
Copy Markdown
Contributor

Auto-generated from the updated HotData OpenAPI spec.
Source: https://github.com/hotdata-dev/www.hotdata.dev/pull/283

@hotdata-automation
hotdata-automation Bot requested a review from a team as a code owner July 28, 2026 21:01
@hotdata-automation
hotdata-automation Bot requested review from anoop-narang and removed request for a team July 28, 2026 21:01
Comment thread src/apis/query_api.rs
};
req_builder = req_builder.header("X-Session-Id", value);
};
if let Some(token) = configuration.resolve_bearer_token().await {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 path Client::query actually uses) → sends X-Session-Id
  • src/client.rs:468 (Client::submit_query) → sends X-Session-Id
  • src/arrow.rs:305 (apply_apikey_headers, used by get_result_arrow / stream_result_arrow) → sends X-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:

  1. If session auth is genuinely gone from the API — finish the removal in the same PR: drop ClientBuilder::session_id, SESSION_ID_HEADER, the HOTDATA_SESSION_ID lookup, the SESSION_ID_HEADER entries in src/query.rs / src/arrow.rs / src/client.rs::submit_query, the now-wrong arrow.rs tests and doc comments that say "mirroring the generated get_result" (src/arrow.rs:301, src/arrow.rs:496, src/query.rs:639), and the X-Session-Id rationale in .openapi-generator-templates/README.md:7. Record it in the changelog as Breaking since session_id() is public API.
  2. If session scoping is still enforced server-side — the spec change is wrong and this regen should not ship.

Comment thread CHANGELOG.md

### Changed

- chore(api): stop injecting SessionId into merged spec

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review

Blocking Issues

  • src/apis/query_api.rs:57, src/apis/results_api.rs:76, src/apis/results_api.rs:150 — removing the X-Session-Id emission leaves no session-header emission anywhere in src/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: one Client::query() call now submits with the session header and then polls GET /v1/results/{id} without it, and JSON vs Arrow result fetches disagree on scoping.
  • src/client.rs:126ClientBuilder::session_id and the HOTDATA_SESSION_ID env var are public API. They still install the key and still document "Installed as the X-Session-Id header when present", which is now only true on the hand-written paths. If the backend still scopes results by session, session-scoped get_result / list_results calls are silently no longer scoped.
  • Stale statements left behind: src/arrow.rs:301 and src/query.rs:639 say the helpers mirror the generated ops' isKeyInHeader blocks; the test at src/arrow.rs:496 asserts behavior described as "matching the generated get_result". Neither holds after this change. .openapi-generator-templates/README.md:7 still cites X-Session-Id as the reason for the template override.

Action Required

Pick one and land it in this PR:

  1. Session auth is gone from the API → finish the removal: drop ClientBuilder::session_id, SESSION_ID_HEADER, the HOTDATA_SESSION_ID lookup, the session entries in src/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, since session_id() is public.
  2. 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.

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.

0 participants