Skip to content

DelegateHandler instead of Retry Interface#2

Closed
lemillermicrosoft wants to merge 8 commits intomainfrom
u/lemiller/retryafter_delegatinghandler
Closed

DelegateHandler instead of Retry Interface#2
lemillermicrosoft wants to merge 8 commits intomainfrom
u/lemiller/retryafter_delegatinghandler

Conversation

@lemillermicrosoft
Copy link
Copy Markdown
Owner

This commit refactors the HTTP retry logic for the kernel and its services by using DelegatingHandler subclasses instead of custom interfaces. It also adds a new HttpRetryConfig class that can be used to configure the default retry parameters for the kernel. The commit uses the Polly library and its extensions to implement the retry policies, and provides some examples of different retry handlers. The commit also updates the tests, the documentation, and the OpenAI clients to reflect these changes.

Comment thread dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs Outdated
@lemillermicrosoft lemillermicrosoft force-pushed the u/lemiller/retryafter_delegatinghandler branch 2 times, most recently from 389a536 to c492c9b Compare March 9, 2023 07:22
@lemillermicrosoft lemillermicrosoft changed the base branch from u/lemiller/retryafter to main March 9, 2023 07:22
Comment thread dotnet/nuget/nuget-package.props Outdated
Comment thread dotnet/src/SemanticKernel/KernelBuilder.cs Outdated
Comment thread dotnet/src/SemanticKernel/Reliability/DefaultHttpRetryHandler.cs Outdated
Comment thread dotnet/src/SemanticKernel/Configuration/KernelConfig.cs Outdated
Comment thread dotnet/src/SemanticKernel/Reliability/DefaultHttpRetryHandler.cs Outdated
Comment thread dotnet/src/SemanticKernel/Configuration/KernelConfig.cs Outdated
Comment thread samples/dotnet/kernel-syntax-examples/Example08_RetryHandler.cs Outdated
Comment thread dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs Outdated
…etry policy used by the semantic kernel and its extensions. The main changes are:

- Replacing the IRetryMechanism interface with the IDelegatingHandlerFactory interface, which allows more flexibility and control over the retry logic for HTTP requests.
- Adding a new DefaultHttpRetryHandler class that inherits from DelegatingHandler and implements the default retry policy using Polly. The policy handles retryable status codes and exceptions, respects the RetryAfter header from the server, and uses exponential backoff if configured.
- Adding a new NullHttpRetryHandler class that implements a no-retry policy.
- Adding a new HttpRetryConfig class that defines the default retry configuration for the IHttpRetryPolicy.
- Adding a new HttpHandlerFactory property to the KernelConfig class, which allows users to specify a custom retry handler factory for the kernel. The class also exposes a SetHttpHandlerFactory and a SetDefaultHttpRetryConfig method to configure these properties.
- Updating the OpenAI services to use the HttpHandlerFactory from the KernelConfig, and adding a new parameter to their constructors.
- Adding unit tests for the DefaultHttpRetryHandler and NullHttpRetryHandler classes, covering various scenarios of retrying and not retrying HTTP requests.
- Adding a new file Example08_RetryHandler.cs that demonstrates how to use different retry policies when making HTTP requests with the semantic kernel.
- Removing the PassThroughWithoutRetry class and the Example08_RetryMechanism.cs file, as they are no longer needed.
- Properly disposing of the retry handler in the OpenAIClientAbstract class.
- Adding a CancellationToken parameter to the CompleteAsync method of the ITextCompletion interface and its implementations, allowing the caller to cancel the request if needed.
- Refactoring the OpenAIClientAbstract class to use a delegating handler factory for injecting retry logic, and updating the AzureOpenAIClientAbstract class accordingly.
- Modifying some documentation and tests to reflect the changes.
- Updating the project file to allow Moq to access internal types for testing purposes.
- Fixing some typos and formatting issues in the code.
Summary: This commit changes the version prefix of all nuget packages from 0.9 to 0.8 in the nuget-package.props file. This is done to align with the current release cycle and avoid confusion with the previous versions.
Summary: This commit makes several changes to the HTTP retry handler logic and logging for the Semantic Kernel project. The main changes are:

- Pass the logger instance to the retry handler factory instead of the constructor, to allow different loggers for different clients.
- Change the log level from warning to error when the max retry count or time is reached, to indicate a more severe problem.
- Remove the TimeoutException and WebException from the list of retryable exception types, since they are already handled by the HttpRequestException.
- Rename the WithRetryHandler method to WithRetryHandlerFactory, to avoid confusion with the DelegatingHandler class.
- Fix a typo in the CloneAsync method comment.
… in the RepoUtils project. It makes the following changes:

- Simplifies the creation of retry handler factories by passing the logger as a parameter instead of a constructor dependency. This avoids creating multiple logger instances for each factory.
- Removes the unused TimeoutException and WebException types from the retry handler tests, since they are no longer used by the HttpClient.
- Changes the logging format to use milliseconds instead of msecs for consistency and clarity. It also fixes the formatting of the time span values.
- Adds some missing log messages for the NoRetryPolicy and the DefaultHttpRetryHandler.
- Lowers the minimum level for the console logger to show all messages, and adds a filter for the System namespace to only show warnings or higher. This makes the console output more informative and less noisy.
@lemillermicrosoft lemillermicrosoft force-pushed the u/lemiller/retryafter_delegatinghandler branch from e925c65 to e15cae2 Compare March 10, 2023 20:47
@lemillermicrosoft
Copy link
Copy Markdown
Owner Author

Published in microsoft#58

dehoward pushed a commit that referenced this pull request Jun 1, 2023
The translation feature in the Book Creator Sample App doesn't work as expected, due to a bug where a wrong target language is passed in.
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.

1 participant