Skip to content

Conversation

@maxxgx
Copy link
Contributor

@maxxgx maxxgx commented Oct 27, 2025

πŸ“ Description

Yield ServerSentEvent instead of raw string.
To be merged after #3053

✨ Changes

Select what type of change your PR is:

  • πŸš€ New feature (non-breaking change which adds functionality)
  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • πŸ”„ Refactor (non-breaking change which refactors the code base)
  • ⚑ Performance improvements
  • 🎨 Style changes (code style/formatting)
  • πŸ§ͺ Tests (adding/modifying tests)
  • πŸ“š Documentation update
  • πŸ“¦ Build system changes
  • 🚧 CI/CD configuration
  • πŸ”§ Chore (general maintenance)
  • πŸ”’ Security update
  • πŸ’₯ Breaking change (fix or feature that would cause existing functionality to not work as expected)

βœ… Checklist

Before you submit your pull request, please make sure you have completed the following steps:

  • πŸ“š I have made the necessary updates to the documentation (if applicable).
  • πŸ§ͺ I have written tests that support my changes and prove that my fix is effective or my feature works (if applicable).
  • 🏷️ My PR title follows conventional commit format.

For more information about code review checklists, see the Code Review Checklist.

Copilot AI review requested due to automatic review settings October 27, 2025 11:28
@maxxgx maxxgx requested a review from samet-akcay as a code owner October 27, 2025 11:28
Copy link
Contributor

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 refactors the job log streaming implementation to use Server-Sent Events (SSE) with proper types from the sse_starlette library. Instead of yielding raw strings, the code now yields ServerSentEvent objects and uses EventSourceResponse for the FastAPI endpoint.

Key Changes

  • Updated JobService.stream_logs() to yield ServerSentEvent objects with proper type hints
  • Replaced StreamingResponse with EventSourceResponse in the job logs endpoint
  • Updated tests to reflect the new SSE-based streaming behavior

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
application/backend/src/services/job_service.py Added SSE types, return type annotation, and modified generator to yield ServerSentEvent objects with stripped newlines
application/backend/src/api/endpoints/job_endpoints.py Changed response type from StreamingResponse to EventSourceResponse and removed explicit media type
application/backend/tests/unit/services/test_job_service.py Updated test to extract data from ServerSentEvent objects and removed newlines from mock log data
application/backend/tests/unit/endpoints/test_jobs.py Updated mock generator to yield ServerSentEvent objects and adjusted assertion for SSE format overhead

πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>
Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>
@codecov-commenter
Copy link

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

Codecov Report

βœ… All modified and coverable lines are covered by tests.

πŸ“’ Thoughts on this report? Let us know!

@maxxgx maxxgx merged commit 71ca2c1 into open-edge-platform:feature/geti-inspect Oct 28, 2025
14 of 17 checks passed
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.

3 participants