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

Add support for non-static native functions with external references #937

Merged
merged 7 commits into from
May 11, 2023

Conversation

dluc
Copy link
Collaborator

@dluc dluc commented May 11, 2023

Motivation and Context

When registering local functions that reference external variables (gRPC scenario) SKFunction throws an exception because Delegate.CreateDelegate returns NULL.

Description

Fix the delegate type detection to support the case of non-static functions referencing external functions + Unit tests.

@dluc dluc requested a review from adrianwyatt May 11, 2023 04:56
@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels May 11, 2023
@dluc dluc requested a review from shawncal May 11, 2023 04:56
@dluc
Copy link
Collaborator Author

dluc commented May 11, 2023

Please test thoroughly before merging. Unit tests should cover all the scenarios, but I would recommend testing manually gRPC, CoPilot chat and syntax examples.

@dluc dluc added the PR: ready for review All feedback addressed, ready for reviews label May 11, 2023
@dluc dluc force-pushed the dluc151nativefunctions branch 2 times, most recently from caf7f97 to 2be4487 Compare May 11, 2023 05:32
shawncal
shawncal previously approved these changes May 11, 2023
adrianwyatt
adrianwyatt previously approved these changes May 11, 2023
@adrianwyatt
Copy link
Contributor

"Could not load file or assembly 'Microsoft.Bcl.AsyncInterfaces, Version=7.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" pops up in CopilotChat and the KernelSyntaxExamples. Looking for a good place to add the NuGet...

@adrianwyatt
Copy link
Contributor

"Could not load file or assembly 'Microsoft.Bcl.AsyncInterfaces, Version=7.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" pops up in CopilotChat and the KernelSyntaxExamples. Looking for a good place to add the NuGet...

Fixing with #955

@adrianwyatt adrianwyatt enabled auto-merge (squash) May 11, 2023 16:19
@adrianwyatt adrianwyatt merged commit f5730da into microsoft:main May 11, 2023
shawncal pushed a commit to johnoliver/semantic-kernel that referenced this pull request May 19, 2023
…icrosoft#937)

### Motivation and Context

When registering local functions that reference external variables (gRPC
scenario) SKFunction throws an exception because
`Delegate.CreateDelegate` returns NULL.

### Description

Fix the delegate type detection to support the case of non-static
functions referencing external functions + Unit tests.

---------

Co-authored-by: Adrian Bonar <[email protected]>
Co-authored-by: Adrian Bonar (HE/HIM) <[email protected]>
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
…icrosoft#937)

### Motivation and Context

When registering local functions that reference external variables (gRPC
scenario) SKFunction throws an exception because
`Delegate.CreateDelegate` returns NULL.

### Description

Fix the delegate type detection to support the case of non-static
functions referencing external functions + Unit tests.

---------

Co-authored-by: Adrian Bonar <[email protected]>
Co-authored-by: Adrian Bonar (HE/HIM) <[email protected]>
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.

4 participants