Skip to content

feat(BA-1453): Add missing session status_history API #4543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
May 29, 2025
Merged

feat(BA-1453): Add missing session status_history API #4543

merged 4 commits into from
May 29, 2025

Conversation

jopemachine
Copy link
Member

@jopemachine jopemachine commented May 29, 2025 •
edited by github-actions bot
Loading

resolves #4518 (BA-1453)

Tested manually.

Example test code

async def test():
    async with AsyncSession() as session:
        r = await session.SessionTemplate.list_templates()
        print(r)

Result

{'result': {'PENDING': '2025-05-29T02:11:56.602497+00:00', 'RUNNING': '2025-05-29T02:11:59.551399+00:00', 'CREATING': '2025-05-29T02:11:57.351795+00:00', 'PREPARED': '2025-05-29T02:11:57.342391+00:00', 'PREPARING': '2025-05-29T02:11:57.306649+00:00', 'SCHEDULED': '2025-05-29T02:11:57.297149+00:00'}}

Checklist: (if applicable)

  • Mention to the original issue

📚 Documentation preview 📚: https://sorna--4543.org.readthedocs.build/en/4543/

📚 Documentation preview 📚: https://sorna-ko--4543.org.readthedocs.build/ko/4543/

@github-actions github-actions bot added size:M 30~100 LoC comp:manager Related to Manager component labels May 29, 2025
@jopemachine jopemachine changed the title feat: Add missing session status_history API feat(BA-1453): Add missing session status_history API May 29, 2025
@jopemachine jopemachine marked this pull request as ready for review May 29, 2025 02:28
Copilot
Copilot AI reviewed May 29, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new session API endpoint for retrieving a session’s status history.

  • Implements get_status_history in the service layer.
  • Registers and wires up the corresponding processor.
  • Exposes a GET /{session_name}/status-history endpoint in the HTTP API.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/ai/backend/manager/services/session/service.py Add get_status_history method and imports
src/ai/backend/manager/services/session/processors.py Register get_status_history ActionProcessor
src/ai/backend/manager/services/session/actions/get_status_history.py Define action/result dataclasses for status history
src/ai/backend/manager/api/session.py Add HTTP handler and route for /status-history
changes/4543.feature.md Document the new status history endpoint
Comments suppressed due to low confidence (1)

src/ai/backend/manager/api/session.py:1633

  • There are no tests covering the new GET /{session_name}/status-history endpoint. Consider adding unit or integration tests to verify authorization, parameter parsing, and the returned payload format.
async def get_status_history(request: web.Request, params: Any) -> web.Response:

src/ai/backend/manager/api/session.py
result = await root_ctx.processors.session.get_status_history.wait_for_complete(
GetStatusHistoryAction(
session_name=session_name,
owner_access_key=request["keypair"]["access_key"],
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

The owner_access_key passed to the action is hardcoded from request["keypair"] instead of using the owner_access_key variable returned by get_access_key_scopes. Replace this with owner_access_key=owner_access_key.

Suggested change
owner_access_key=request["keypair"]["access_key"],
owner_access_key=owner_access_key,

Copilot uses AI. Check for mistakes.

changes/4543.feature.md
@@ -0,0 +1 @@
Add missing `GET /status_history` endpoint to the session REST API
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

The feature doc uses /status_history (underscore), but the code registers /status-history (hyphen). Update the documentation to reflect the actual route.

Copilot uses AI. Check for mistakes.

@github-actions github-actions bot added size:L 100~500 LoC area:docs Documentations and removed size:M 30~100 LoC labels May 29, 2025
@jopemachine jopemachine force-pushed the feat/add_missing_session_status_history_api branch from 8dcd99f to 42840bb Compare May 29, 2025 06:20
@github-actions github-actions bot added size:M 30~100 LoC and removed size:L 100~500 LoC labels May 29, 2025
@jopemachine jopemachine requested a review from HyeockJinKim May 29, 2025 06:21
@jopemachine @lablup-octodog
chore: update api schema dump
2e141ed 
Co-authored-by: octodog <mu001@lablup.com>
@github-actions github-actions bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels May 29, 2025
@jopemachine
Copy link
Member Author

This PR is primarily for adding a new feature, so it seems appropriate not to backport it for now, even if there is a dangling SDK method.

@HyeockJinKim HyeockJinKim added this pull request to the merge queue May 29, 2025
Merged via the queue into main with commit 9d51327 May 29, 2025
29 checks passed
@HyeockJinKim HyeockJinKim deleted the feat/add_missing_session_status_history_api branch May 29, 2025 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@HyeockJinKim HyeockJinKim HyeockJinKim approved these changes

Assignees

@jopemachine jopemachine

Labels
area:docs Documentations comp:manager Related to Manager component size:L 100~500 LoC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Session.get_status_history API not found error
2 participants
@jopemachine @HyeockJinKim