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.

Sign up for GitHub

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

Jump to bottom

Add request timeout configurable #1

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Add request timeout configurable #1

wants to merge 16 commits into from

Conversation

TaoChenOSU
Copy link
Owner

Motivation and Context

Description

Contribution Checklist

lemillermicrosoft
lemillermicrosoft reviewed Mar 7, 2023
View reviewed changes
dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs
string responseJson;

try
{
using HttpContent content = new StringContent(requestBody, Encoding.UTF8, MediaTypeNames.Application.Json);
HttpResponseMessage response = await this.HTTPClient.PostAsync(url, content);
HttpResponseMessage response = await this.HTTPClient.PostAsync(url, content, cancellationSource.Token);
Copy link
Collaborator

@lemillermicrosoft lemillermicrosoft Mar 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was going to plumb the CancellationTokenSource from the SKContext of the execution as well, so eventually this might look like:

        using var cancellationSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
        cancellationSource.CancelAfter(TimeSpan.FromSeconds(httpTimeoutInSeconds));

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is mostly a note for myself or whoever ends up doing the merge resolution here :)

Copy link
Owner Author

@TaoChenOSU TaoChenOSU Mar 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If we use a cancellation token that is passed in, we can't really tell if CancelAfter() has been called on the source. This could cause accidental override or infinite timeout on the http client.

awharrison-28
awharrison-28 requested changes Mar 8, 2023
View reviewed changes
dotnet/src/SemanticKernel.Test/Orchestration/SKFunctionTests1.cs Outdated Show resolved Hide resolved
dotnet/src/SemanticKernel.Test/Orchestration/SKFunctionTests1.cs Outdated Show resolved Hide resolved
dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs Outdated Show resolved Hide resolved
dotnet/src/SemanticKernel/KernelExtensions/InlineFunctionsDefinitionExtension.cs Outdated Show resolved Hide resolved
dotnet/src/SemanticKernel/Orchestration/SKFunctionExtensions.cs Show resolved Hide resolved
dotnet/src/SemanticKernel/Orchestration/SKFunctionExtensions.cs Show resolved Hide resolved
dotnet/src/SemanticKernel/Orchestration/SKFunctionExtensions.cs Show resolved Hide resolved
dotnet/src/SemanticKernel/Orchestration/SKFunctionExtensions.cs Show resolved Hide resolved
dotnet/src/SemanticKernel/Orchestration/SKFunctionExtensions.cs Show resolved Hide resolved
dotnet/src/SemanticKernel/SemanticFunctions/PromptTemplateConfig.cs Outdated Show resolved Hide resolved
@TaoChenOSU TaoChenOSU force-pushed the users/taochen/SKFunction_http_timeout branch from a174142 to b2df94d Compare March 9, 2023 01:40
@TaoChenOSU TaoChenOSU force-pushed the users/taochen/SKFunction_http_timeout branch from b2df94d to 7b598e9 Compare March 9, 2023 18:17
@TaoChenOSU TaoChenOSU force-pushed the users/taochen/SKFunction_http_timeout branch from c00c3d4 to ab15c11 Compare March 9, 2023 19:24
TaoChenOSU pushed a commit that referenced this pull request Jul 20, 2023
@teresaqhoang
Merge pull request #1 from teresaqhoang/ui-simplification-changes
c8ac553 
Copilot Chat - Enabling Feature Flags + Simple UI
TaoChenOSU pushed a commit that referenced this pull request Nov 14, 2023
@markwallace-microsoft
.Net: Add support for arbitrary service attributes - Option #1 (micro…
5e5b4a4 
…soft#3415)

### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
TaoChenOSU pushed a commit that referenced this pull request Nov 27, 2023
@RogerBarreto
.Net Kernel and ISKFunction Streaming Option #1 (Phase 1) (microsoft#…
b9c742d 
…3496)

## Context and Problem Statement

Resolves microsoft#1649
Resolves microsoft#1298 

It is quite common in co-pilot implementations to have a streamlined
output of messages from the LLM (large language models) and currently
that is not possible while using ISKFunctions.InvokeAsync or
Kernel.RunAsync methods, which enforces users to work around the Kernel
and Functions to use `ITextCompletion` and `IChatCompletion` services
directly as the only interfaces that currently support streaming.

Currently streaming is a capability that not all providers do support
and this as part of our design we try to ensure the services will have
the proper abstractions to support streaming not only of text but be
open to other types of data like images, audio, video, etc.

Needs to be clear for the sk developer when he is attempting to get
streaming data.

## Decision Drivers

1. The sk developer should be able to get streaming data from the Kernel
and Functions using Kernel.RunAsync or ISKFunctions.InvokeAsync methods

2. The sk developer should be able to get the data in a generic way, so
the Kernel and Functions can be able to stream data of any type, not
limited to text.

3. The sk developer when using streaming from a model that does not
support streaming should still be able to use it with only one streaming
update representing the whole data.

## User Experience Goal

```csharp
//(providing the type at as generic parameter)

// Getting a Raw Streaming data from Kernel
await foreach(string update in kernel.StreamingRunAsync<byte[]>(function, variables))

// Getting a String as Streaming data from Kernel
await foreach(string update in kernel.StreamingRunAsync<string>(function, variables))

// Getting a StreamingResultUpdate as Streaming data from Kernel
await foreach(StreamingResultUpdate update in kernel.StreamingRunAsync<StreamingResultUpdate>(variables, function))
// OR
await foreach(StreamingResultUpdate update in kernel.StreamingRunAsync(function, variables)) // defaults to Generic above)
{
    Console.WriteLine(update);
}
```

## Out of Scope

- Streaming with plans will not be supported in this phase. Attempting
to do so will throw an exception.
- Kernel streaming will not support multiple functions (pipeline).

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
glahaye pushed a commit that referenced this pull request Dec 13, 2023
@SergeyMenshykh @markwallace-microsoft
.Net: Simplify memory namespaces (microsoft#4230)
0095578 
Closes task `#1` of the issue -
microsoft#3151

### Description

The `Memory` keyword is removed from the namespaces to make them
slightly shorter and aligned with AI connector namespaces:
- `Microsoft.SemanticKernel.Connectors.Memory.Chroma` ->
`Microsoft.SemanticKernel.Connectors.Chroma`
- `Microsoft.SemanticKernel.Connectors.Memory.AzureAISearch` ->
`Microsoft.SemanticKernel.Connectors.AzureAISearch`
- same for Kusto, Milvus, etc

The `MemoryBuilder`, `VolatileMemoryStore` classes that live in the
`Plugins.Memory` project under the
`Microsoft.SemanticKernel.Plugins.Memory` namespace are moved to
`Microsoft.SemanticKernel.Memory` namespace where they naturally belong
to.

---------

Co-authored-by: Mark Wallace <127216156+markwallace-microsoft@users.noreply.github.com>
TaoChenOSU pushed a commit that referenced this pull request Jun 3, 2024
@SandraAhlgrimm @johnoliver
Squashed commit of the following:
1b2e2fb 
commit 538774d
Merge: 1a5f744 dcd7e14
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Thu Feb 8 08:50:21 2024 +0100

    Merge branch 'main' into update-modelid

commit 1a5f744
Merge: 699eec8 7e43195
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Wed Feb 7 17:35:02 2024 +0100

    Merge branch 'main' into update-modelid

commit 699eec8
Merge: 60cf261 7e43195
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Wed Feb 7 10:30:34 2024 +0100

    Merge branch 'main' into update-modelid

commit 60cf261
Merge: 2f726c2 3a7d3bd
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Wed Feb 7 10:22:51 2024 +0100

    Merge branch 'main' into update-modelid

commit 2f726c2
Merge: 5bdbbd6 59508ca
Author: John Oliver <1615532+johnoliver@users.noreply.github.com>
Date:   Tue Feb 6 13:02:30 2024 +0000

    Merge branch 'main' into update-modelid

commit 5bdbbd6
Merge: ffcf68c eed5193
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Fri Feb 2 22:44:25 2024 +0100

    Merge branch 'main' into update-modelid

commit ffcf68c
Merge: 3a34f5d 52e317a
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Fri Feb 2 12:15:47 2024 +0100

    Merge branch 'main' into update-modelid

commit 3a34f5d
Merge: 7933961 e582951
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Feb 2 12:15:13 2024 +0100

    Merge remote-tracking branch 'origin/update-modelid' into update-modelid

commit 7933961
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Feb 2 12:14:50 2024 +0100

    make Deployment Name optional and set modelid with default as requested in review

commit b74843e
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Feb 2 12:13:39 2024 +0100

    make Deployment Name optional and set modelid with default as requested in review

commit e582951
Merge: f7f1664 5953beb
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Fri Jan 26 22:56:33 2024 +0100

    Merge branch 'main' into update-modelid

commit f7f1664
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 26 22:51:38 2024 +0100

    spring boot starter gets tested with samples

commit be6cf95
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 26 12:46:23 2024 +0100

    Need to use JDK 11 to build for JDK 8

commit 1af0f61
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 26 12:43:16 2024 +0100

    Need to use JDK 11 to build for JDK 8

commit 5bebe6c
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 26 12:24:46 2024 +0100

    move starter to example package

commit 2f8a839
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 26 10:14:20 2024 +0100

    switch back to jdk 8 and 17 as required by PM

commit 9fdf375
Merge: 1e66ccb 038ca45
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 26 10:08:32 2024 +0100

    Merge branch 'main' of https://github.com/SandraAhlgrimm/semantic-kernel into update-modelid

    # Conflicts:
    #	.github/workflows/java-build.yml

commit 038ca45
Merge: 21d161a e0ccee0
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Fri Jan 26 10:03:52 2024 +0100

    Merge branch 'microsoft:main' into main

commit 21d161a
Merge: a402026 4445327
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Fri Jan 26 09:31:59 2024 +0100

    Merge pull request #1 from bbenz/main

    bbenz Update java-build.yml

commit a402026
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Fri Jan 26 09:31:19 2024 +0100

    Update java-build.yml

commit 4445327
Author: Brian Benz <bbenz@microsoft.com>
Date:   Thu Jan 25 16:07:29 2024 -0800

    bbenz Update java-build.yml

    Set JDK to 11 or 17

commit 1e66ccb
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Thu Jan 25 21:32:07 2024 +0100

    Update java-build.yml

    update workflow as @bbenz suggested

commit b69342a
Merge: 79627be e0ccee0
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Thu Jan 25 21:30:42 2024 +0100

    Merge branch 'microsoft:main' into update-modelid

commit 79627be
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Thu Jan 25 18:22:56 2024 +0100

    jdk 17 for spring boot starter

commit 14e17ad
Merge: 4570ad2 00fbab2
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Wed Jan 24 21:41:47 2024 +0100

    Merge remote-tracking branch 'origin/update-modelid' into update-modelid

commit 4570ad2
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Wed Jan 24 21:41:14 2024 +0100

    cleaned up and ready for PR

commit 00fbab2
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Wed Jan 24 21:39:57 2024 +0100

    Update java-integration-tests.yml

commit f5baf9e
Merge: f76a4b0 f959256
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Wed Jan 24 21:33:28 2024 +0100

    Merge branch 'microsoft:main' into update-modelid

commit f76a4b0
Merge: a8cf41e e4874bb
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Mon Jan 22 16:08:03 2024 +0100

    Merge branch 'microsoft:main' into update-modelid

commit a8cf41e
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Mon Jan 22 16:03:22 2024 +0100

    cleaned up and ready for PR

commit 8d3d26a
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Thu Jan 18 22:43:40 2024 +0100

    maven still needs a cleanup

commit f811bc3
Merge: 43b0a7f c87e45e
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Wed Jan 17 22:27:39 2024 +0100

    Merge remote-tracking branch 'origin/update-modelid' into update-modelid

commit c87e45e
Merge: 355eb68 fe23d41
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Wed Jan 17 22:20:29 2024 +0100

    Merge branch 'microsoft:main' into update-modelid

commit 43b0a7f
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Wed Jan 17 22:19:31 2024 +0100

    add tests, maven needs a cleanup

commit 355eb68
Merge: 4277528 f9bef63
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Tue Jan 16 15:40:51 2024 +0100

    Merge branch 'microsoft:main' into update-modelid

commit 4277528
Merge: f2cab02 9db47e5
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Mon Jan 8 14:22:56 2024 +0100

    Merge branch 'microsoft:main' into update-modelid

commit f2cab02
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 5 16:58:21 2024 +0100

    update maven dependencies, test needs to be finished

commit ec31872
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 5 15:52:49 2024 +0100

    add spring boot starter

commit ff87caa
Merge: c1ccfe0 11369bf
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 5 13:16:21 2024 +0100

    Merge remote-tracking branch 'origin/update-modelid' into update-modelid

commit c1ccfe0
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 5 13:16:08 2024 +0100

    add spring boot starter

commit 11369bf
Merge: ac73cb8 833a2d7
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Fri Jan 5 13:13:37 2024 +0100

    Merge branch 'microsoft:main' into update-modelid

commit ac73cb8
Merge: c1f4ef8 4b9dbed
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 5 13:11:50 2024 +0100

    Merge branch 'update-modelid' of https://github.com/SandraAhlgrimm/semantic-kernel into update-modelid

commit c1f4ef8
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Jan 5 13:06:20 2024 +0100

    add spring boot starter

commit 4b9dbed
Merge: 9da9b38 aa62b14
Author: John Oliver <1615532+johnoliver@users.noreply.github.com>
Date:   Thu Jan 4 12:46:38 2024 +0000

    Merge branch 'main' into update-modelid

commit 53d4bf7
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Wed Jan 3 20:14:32 2024 +0100

    add spring boot starter

commit 9da9b38
Merge: a3782cf 79908b0
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Wed Jan 3 13:24:58 2024 +0100

    Merge branch 'main' into update-modelid

commit a3782cf
Merge: a693099 240a2cb
Author: Sandra Ahlgrimm <sandra.kriemann@gmail.com>
Date:   Fri Dec 22 11:40:14 2023 +0100

    Merge branch 'microsoft:main' into update-modelid

commit a693099
Author: SandraAhlgrimm <sakriema@microsoft.com>
Date:   Fri Dec 22 11:38:59 2023 +0100

    update modelid to the available davinci-002 on Azure Open AI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awharrison-28 awharrison-28 awharrison-28 requested changes

@lemillermicrosoft lemillermicrosoft Awaiting requested review from lemillermicrosoft

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TaoChenOSU @lemillermicrosoft @awharrison-28