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

Decouple AzureOpenAIChatCompletionService from Concrete Implementation Using ITextGenerationService #801

Conversation

mirapalheta
Copy link

Motivation for the Change

The primary motivation for this change is to enhance the modularity, flexibility, and testability of the AzureOpenAIChatCompletionService. By depending on an interface (ITextGenerationService) rather than a concrete implementation (AzureOpenAITextGenerator), we can make the service more adaptable to future changes and varying requirements in text generation logic.

  1. Decoupling: Introducing an interface decouples the business logic of AzureOpenAIChatCompletionService from the specific implementation details of AzureOpenAITextGenerator, promoting a more maintainable code structure.
  2. Testability: This change allows for easier unit testing by enabling the use of mock or stub implementations of ITextGenerationService for testing purposes, without relying on the actual Azure service.
  3. Flexibility: With the service depending on an interface, it becomes easier to switch implementations if needed, whether that’s to use a different text generation service or to implement additional features without impacting the service's core functionality.

Context

In the original setup, the AzureOpenAIChatCompletionService was tightly coupled to a specific text generation implementation provided by AzureOpenAITextGenerator. As the application develops, the need for integrating other text generation models or services may arise. This change allows for a more robust architecture that can accommodate such adaptations without major refactoring efforts.

High-Level Description

The AzureOpenAIChatCompletionService now utilizes an interface, ITextGenerationService, for its text generation operations. This redesign includes:

  • Interface Definition: The ITextGenerationService interface defines the contract for any text generation service. It outlines the methods that must be implemented, providing a common ground for various text generation logic implementations.

  • Implementation Variability: The AzureOpenAITextGenerator can still serve as a concrete implementation of this interface, but now it's just one of many potential implementations. Other text generation services can easily be integrated in the future by creating new classes that implement ITextGenerationService.

  • Dependency Injection: The AzureOpenAIChatCompletionService will now receive an instance of ITextGenerationService through dependency injection, allowing for greater control over how services are instantiated and managed within the application.

This change supports a more scalable and maintainable architecture, aligning with best practices in software design such as dependency inversion and interface segregation.

@dluc
Copy link
Collaborator

dluc commented Sep 25, 2024

There might be some confusion around the constructors of the AzureOpenAITextGenerator class.

To work with generic implementations of ITextGenerationService, it’s recommended to use the SemanticKernelTextGenerator class, which can also be accessed through the WithSemanticKernelTextGenerationService memory builder extension method.

AzureOpenAITextGenerator is specifically designed to integrate with Semantic Kernel’s AzureOpenAIChatCompletionService.

When using this constructor:

public AzureOpenAITextGenerator(
        AzureOpenAIConfig config,
        ITextTokenizer? textTokenizer = null,
        ILoggerFactory? loggerFactory = null,
        HttpClient? httpClient = null)

the class internally creates an instance of AzureOpenAIChatCompletionService based on predefined logic.

For scenarios where there’s a need to create that instance differently (e.g. with custom authentication, resilience strategies, telemetry, etc.) this alternative constructor can be used:

public AzureOpenAITextGenerator(
        AzureOpenAIConfig config,
        AzureOpenAIChatCompletionService skClient,
        ITextTokenizer? textTokenizer = null,
        ILoggerFactory? loggerFactory = null)

@dluc dluc closed this Sep 25, 2024
@mirapalheta
Copy link
Author

mirapalheta commented Sep 26, 2024

For scenarios where there’s a need to create that instance differently (e.g. with custom authentication, resilience strategies, telemetry, etc.) this alternative constructor can be used:

public AzureOpenAITextGenerator(
        AzureOpenAIConfig config,
        AzureOpenAIChatCompletionService skClient,
        ITextTokenizer? textTokenizer = null,
        ILoggerFactory? loggerFactory = null)

@deluc: The proposed change is specifically in this constructor, but as it is depending on AzureOpenAIChatCompletionService implementation, instead of the interface ITextGenerationService, it does not allow flexibility on its usage.

In our scenario we have custom implementation around AzureOpenAIChatCompletionService, using our own class to proxy the calls to it, but the latest changes to KM, with the introduction of this dependency to a concrete class, broke our implementation.

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.

2 participants