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

.Net: Adding IKernel property the SKContext - Phase 1 #2846

Merged
merged 13 commits into from Sep 19, 2023

Conversation

RogerBarreto
Copy link
Member

@RogerBarreto RogerBarreto commented Sep 15, 2023

Motivation and Context

⚠️ Breaking Change

Adding Kernel instance as part of the SKContext enable functions to interact with other services available in the kernel from within the functions, including the ability to call other Functions using the same Kernel "context" as described in #2825

This will also allow Kernel.RunAsync to execute and trigger events (Resolves Partially #2324) seamlessly when running plans, plans steps and rendering prompts from templates (template engine) without resorting to implement those events for Plans and Template Engine separately.

This change is also a step forward in making IKernel.RunAsync the main way to Invoke functions.

Description

This first step is only adding Kernel as a required SKContext property,

PS: Other changes will start to harnessing the SKContext.Kernel reference to call Steps and Prompt rendering (thru prompt template engine) moving forward.

Contribution Checklist

@RogerBarreto RogerBarreto added PR: ready for review All feedback addressed, ready for reviews .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel samples PR: breaking change Pull requests that introduce breaking changes labels Sep 15, 2023
@RogerBarreto RogerBarreto requested a review from a team as a code owner September 15, 2023 08:58
@RogerBarreto RogerBarreto self-assigned this Sep 15, 2023
@github-actions github-actions bot changed the title Adding IKernel property the SKContext - Phase 1 .Net: Adding IKernel property the SKContext - Phase 1 Sep 15, 2023
@shawncal shawncal removed the samples label Sep 15, 2023
@SergeyMenshykh
Copy link
Member

@RogerBarreto I would like to understand the rationale behind the change a little better. Did we have a technical discussion? If not, let's have one.

@RogerBarreto RogerBarreto added this to the v1 milestone Sep 15, 2023
Copy link
Member

@markwallace-microsoft markwallace-microsoft left a comment

Choose a reason for hiding this comment

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

LGTM

@RogerBarreto RogerBarreto added this pull request to the merge queue Sep 19, 2023
Merged via the queue into microsoft:main with commit 4921ea9 Sep 19, 2023
18 checks passed
@RogerBarreto RogerBarreto deleted the features/skcontext-kernel branch September 19, 2023 16:30
@@ -93,7 +92,7 @@ public LanguageCalculatorPlugin(IKernel kernel)

try
{
answer = await this._mathTranslator.InvokeAsync(input).ConfigureAwait(false);
answer = await context.Kernel.RunAsync(input).ConfigureAwait(false);
Copy link
Member

Choose a reason for hiding this comment

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

Whoops, I think this was wrong @RogerBarreto

SOE-YoungS pushed a commit to SOE-YoungS/semantic-kernel that referenced this pull request Nov 1, 2023
### Motivation and Context

⚠️ Breaking Change

Adding Kernel instance as part of the SKContext enable functions to
interact with other services available in the kernel from within the
functions, including the ability to call other Functions using the same
Kernel "context" as described in microsoft#2825

This will also allow Kernel.RunAsync to execute and trigger events
(Resolves Partially microsoft#2324) seamlessly when running plans, plans steps
and rendering prompts from templates (template engine) without resorting
to implement those events for Plans and Template Engine separately.

This change is also a step forward in making IKernel.RunAsync the main
way to Invoke functions.

### Description

This first step is only adding Kernel as a required SKContext property, 

PS: Other changes will start to harnessing the SKContext.Kernel
reference to call Steps and Prompt rendering (thru prompt template
engine) moving forward.

### Contribution Checklist

- [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 😄
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: breaking change Pull requests that introduce breaking changes PR: ready for review All feedback addressed, ready for reviews
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants