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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry functionality for OpenAPI skills #779

Merged

Conversation

SergeyMenshykh
Copy link
Member

Motivation and Context

This PR adds two improvements:

  • It extends all ImportOpenAPISkill* and ImportChatGptPluginSkill* methods to accept HTTP retry configuration.
  • It improves HttpClient usage in OpenAPI skills by reusing the same instance for the same set of imported skills.

The first improvement will allow OpenAPI skill to be more resilient against unreliable REST API it tries to access.

The second improvement will decrease number of HttpClient instances OpenAPI skills create. Instead of creating one instance of HttpClient per skill, one instance will be created per set of skills that are imported together by the same Import* method. This is the first step to decrease number of HttpClient created for OpenAPI skills and it's not the final one. Later, when agreed on a proper solution, the HttpClient could be externalized so that client code/hosting app could provide its own instance, if required.

Description

  1. All the ImportOpenAPISkill* and ImportChatGptPluginSkill* methods are extended to accept and respect HttpRetryConfig.
  2. Functionality that creates HttpClient is moved one level up so that it's called once per import rather than for each skill in the import.

Contribution Checklist

@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel samples labels May 2, 2023
@SergeyMenshykh SergeyMenshykh added the PR: ready for review All feedback addressed, ready for reviews label May 2, 2023
@shawncal shawncal force-pushed the retry-for-open-api-skills-v3 branch from 935ed7c to c458684 Compare May 3, 2023 02:07
@shawncal shawncal merged commit 3013b5d into microsoft:main May 3, 2023
11 checks passed
codebrain pushed a commit to searchpioneer/semantic-kernel that referenced this pull request May 16, 2023
### Motivation and Context

This PR adds two improvements:
- It extends all ImportOpenAPISkill* and ImportChatGptPluginSkill*
methods to accept HTTP retry configuration.
- It improves HttpClient usage in OpenAPI skills by reusing the same
instance for the same set of imported skills.

The first improvement will allow OpenAPI skill to be more resilient
against unreliable REST API it tries to access.

The second improvement will decrease number of HttpClient instances
OpenAPI skills create. Instead of creating one instance of HttpClient
per skill, one instance will be created per set of skills that are
imported together by the same Import* method. This is the first step to
decrease number of HttpClient created for OpenAPI skills and it's not
the final one. Later, when agreed on a proper solution, the HttpClient
could be externalized so that client code/hosting app could provide its
own instance, if required.

### Description
1. All the ImportOpenAPISkill* and ImportChatGptPluginSkill* methods are
extended to accept and respect HttpRetryConfig.
2. Functionality that creates HttpClient is moved one level up so that
it's called once per import rather than for each skill in the import.
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
### Motivation and Context

This PR adds two improvements:
- It extends all ImportOpenAPISkill* and ImportChatGptPluginSkill*
methods to accept HTTP retry configuration.
- It improves HttpClient usage in OpenAPI skills by reusing the same
instance for the same set of imported skills.

The first improvement will allow OpenAPI skill to be more resilient
against unreliable REST API it tries to access.

The second improvement will decrease number of HttpClient instances
OpenAPI skills create. Instead of creating one instance of HttpClient
per skill, one instance will be created per set of skills that are
imported together by the same Import* method. This is the first step to
decrease number of HttpClient created for OpenAPI skills and it's not
the final one. Later, when agreed on a proper solution, the HttpClient
could be externalized so that client code/hosting app could provide its
own instance, if required.

### Description
1. All the ImportOpenAPISkill* and ImportChatGptPluginSkill* methods are
extended to accept and respect HttpRetryConfig.
2. Functionality that creates HttpClient is moved one level up so that
it's called once per import rather than for each skill in the import.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: ready for review All feedback addressed, ready for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants