Skip to content

fix: align session expiration behavior across API and SDK - #460

Open
acsoto wants to merge 4 commits into
volcano-sh:mainfrom
acsoto:fix/session-expiration-contract
Open

fix: align session expiration behavior across API and SDK#460
acsoto wants to merge 4 commits into
volcano-sh:mainfrom
acsoto:fix/session-expiration-contract

Conversation

@acsoto

@acsoto acsoto commented Aug 21, 2026

Copy link
Copy Markdown
Member

What type of PR is this?

/kind bug

What this PR does / why we need it:

Aligns the session expiration contract across the Router, Workload Manager, and Python SDK:

  • returns SESSION_NOT_FOUND only when the stored session is missing;
  • treats request ttl as a maximum lifetime capped by spec.maxSessionDuration, independent of the idle spec.sessionTimeout;
  • raises SessionNotFoundError and clears stale session IDs without implicit recreation or retry.

Application-level 404 responses remain ordinary HTTP errors.

Which issue(s) this PR fixes:

Fixes #459

Special notes for your reviewer:

ttl is expressed in seconds. Omitting it preserves the workload CR/default lifetime.

Validated with Go unit tests and vet for the changed packages, 26 Python SDK tests, Ruff, and diff checks. Prepared with AI assistance and manually reviewed and validated.

Does this PR introduce a user-facing change?:

Session expiration now returns a structured error, Python SDKs invalidate stale session IDs, and request-level TTL is enforced within workload limits.

Signed-off-by: Zhou Zihang <z@mcac.cc>
@volcano-sh-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kevin-wangzefeng, yaozengzeng for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Zhou Zihang <z@mcac.cc>
@codecov-commenter

codecov-commenter commented Aug 21, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.13%. Comparing base (5903457) to head (d1fef2d).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
pkg/api/errors.go 75.00% 1 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #460      +/-   ##
==========================================
+ Coverage   59.98%   60.13%   +0.14%     
==========================================
  Files          36       36              
  Lines        3589     3615      +26     
==========================================
+ Hits         2153     2174      +21     
- Misses       1216     1220       +4     
- Partials      220      221       +1     
Flag Coverage Δ
unittests 60.13% <94.73%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@acsoto
acsoto marked this pull request as ready for review August 21, 2026 06:43
Signed-off-by: Zhou Zihang <z@mcac.cc>
Signed-off-by: Zhou Zihang <z@mcac.cc>
@acsoto

acsoto commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

cc @RainbowMango

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify session expiration behavior in API and Python SDK

3 participants