Skip to content
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.

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

Reduce duplication of StreamingResponseHandlers #258

Merged
merged 6 commits into from
Jan 7, 2025

Conversation

CharlesDuboisSAP
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP commented Jan 3, 2025

Context

AI/ai-sdk-java-backlog#111.

Feature scope:

  • Merged both OrchestrationStreamingHandler and OpenAiStreamingHandler into a generic ClientStreamingHandler

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Aligned changes with the JavaScript SDK
  • Documentation updated
  • Release notes updated

@CharlesDuboisSAP CharlesDuboisSAP self-assigned this Jan 3, 2025
@CharlesDuboisSAP CharlesDuboisSAP added the please-review Request to review a pull-request label Jan 6, 2025
* @since 1.1.0
*/
@Slf4j
public class ClientStreamingHandler<D extends StreamedDelta, E extends ClientException> {
Copy link
Contributor

Choose a reason for hiding this comment

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

(Comment)

  • The new class is now public, before it was package-private.
  • It is no longer immutable, with JACKSON being modifiable.

Copy link
Contributor

@newtork newtork left a comment

Choose a reason for hiding this comment

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

Only minor comments and questions. Approach looks very good! 👍

@CharlesDuboisSAP CharlesDuboisSAP enabled auto-merge (squash) January 7, 2025 10:43
@CharlesDuboisSAP CharlesDuboisSAP merged commit 9a0b263 into main Jan 7, 2025
5 checks passed
@CharlesDuboisSAP CharlesDuboisSAP deleted the reduce-duplication branch January 7, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please-review Request to review a pull-request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants