Skip to content

.Net: Replace KernelBuilder with Kernel.CreateBuilder#4096

Merged
stephentoub merged 6 commits intomicrosoft:mainfrom
stephentoub:createbuilder
Dec 9, 2023
Merged

.Net: Replace KernelBuilder with Kernel.CreateBuilder#4096
stephentoub merged 6 commits intomicrosoft:mainfrom
stephentoub:createbuilder

Conversation

@stephentoub
Copy link
Member

@matthewbolanos, @markwallace-microsoft, entirely up to you whether you want to merge or close this; fine with me either way, I'm just opening this to make it easy to decide. Fixes #4033.

This removes KernelBuilder and adds a static CreateBuilder() such that instead of doing:

Kernel kernel = new KernelBuilder().Whatever().Build();

one would now do:

Kernel kernel = Kernel.CreateBuilder().Whatever().Build();

Everything else should be functionally the same.

@stephentoub stephentoub requested a review from a team as a code owner December 8, 2023 04:45
@shawncal shawncal added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels Dec 8, 2023
@github-actions github-actions bot changed the title Replace KernelBuilder with Kernel.CreateBuilder .Net: Replace KernelBuilder with Kernel.CreateBuilder Dec 8, 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 @matthewbolanos do you agree?

@stephentoub stephentoub added this pull request to the merge queue Dec 9, 2023
Merged via the queue into microsoft:main with commit 1c1da87 Dec 9, 2023
@stephentoub stephentoub deleted the createbuilder branch December 9, 2023 14:24
@qihangnet
Copy link

What is the purpose of making such destructive changes repeatedly during the RC phase?
Do you have any misconceptions about RC?

Kevdome3000 pushed a commit to Kevdome3000/semantic-kernel that referenced this pull request Dec 9, 2023
@matthewbolanos, @markwallace-microsoft, entirely up to you whether you
want to merge or close this; fine with me either way, I'm just opening
this to make it easy to decide. Fixes
microsoft#4033.

This removes `KernelBuilder` and adds a static `CreateBuilder()` such
that instead of doing:
```C#
Kernel kernel = new KernelBuilder().Whatever().Build();
```
one would now do:
```C#
Kernel kernel = Kernel.CreateBuilder().Whatever().Build();
```
Everything else should be functionally the same.

---------

Co-authored-by: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com>

(cherry picked from commit 1c1da87)
@markwallace-microsoft
Copy link
Member

What is the purpose of making such destructive changes repeatedly during the RC phase? Do you have any misconceptions about RC?

Hi @qihangnet
apologies for the additional breaking changes. This one was made after an internal review of the API surface and was deemed important for consistency with other .Net API's. The team is working very hard to make sure we provide the best experience for developers. We are on track to have a stable 1.0 API very soon and end the cycle of breaking changes.

@strikene
Copy link

strikene commented Dec 14, 2023

What is the purpose of making such destructive changes repeatedly during the RC phase? Do you have any misconceptions about RC?

Hi @qihangnet apologies for the additional breaking changes. This one was made after an internal review of the API surface and was deemed important for consistency with other .Net API's. The team is working very hard to make sure we provide the best experience for developers. We are on track to have a stable 1.0 API very soon and end the cycle of breaking changes.

After I upgraded from RC3 to RC4, I couldn't even build the kernel properly
CS7069 Reference to type 'IKernelBuilder' claims it is defined in 'Microsoft.SemanticKernel.Core', but it could not be found

image

I kind of wonder if this kind of massive breaking change at this stage of RC4 means that more RC is needed before it can be officially released, and we spent quite a bit of time going from Beta 8 to RC3, even though RC3 still has stack overflows that lead to having to fall back to Beta 8

@stephentoub
Copy link
Member Author

stephentoub commented Dec 14, 2023

What is the purpose of making such destructive changes repeatedly during the RC phase? Do you have any misconceptions about RC?

Hi @qihangnet apologies for the additional breaking changes. This one was made after an internal review of the API surface and was deemed important for consistency with other .Net API's. The team is working very hard to make sure we provide the best experience for developers. We are on track to have a stable 1.0 API very soon and end the cycle of breaking changes.

After I upgraded from RC3 to RC4, I couldn't even build the kernel properly CS7069 Reference to type 'IKernelBuilder' claims it is defined in 'Microsoft.SemanticKernel.Core', but it could not be found

It sounds like something in your solution is still pulling in the RC3 version, such that some assemblies think the interface is defined in the abstractions assembly and some think it's defined in the core assembly.

@qihangnet
Copy link

@strikene, With the release of RC4, there have been significant changes to the namespace and package names related to Connectors. I suggest confirming if you are using the latest version. In my experience, the use of outdated RC3 packages is often the reason for this situation. PR #4230 provides a comprehensive overview of these updates.

I also shared your concern about the significant changes in naming conventions without sufficient documentation or guidance. It seems that there is a difference in understanding between the Semantic Kernel team and the more conservative changes we expect at this stage of the “Release Candidate” (RC); the extent of these changes feels more like the Beta stage rather than the RC stage.

So far, almost all the community-related articles, blogs, video, including official Microsoft documents, have become obsolete, making the experiences of all relevant developers outdated. This reminds me of some things from the Windows Phone era. Many in the community are very concerned about these arbitrary changes, even angry, and some have lost patience and turned to LangChain.

Unfortunately, it looks like these renaming and disruptive changes will continue to occur. This could significantly damage SK's efforts to engage with the community.

@markwallace-microsoft
Copy link
Member

Unfortunately, it looks like these renaming and disruptive changes will continue to occur. This could significantly damage SK's efforts to engage with the community.

There will be no further disruptions in the API surface once we reach v1.0.0 (in the next few days). Our goal is to provide a stable Semantic Kernel that our partners and internal teams can rely on.

@madsbolaris
Copy link
Member

I want to plus one what Mark has shared. The last round of changes during our RC were made explicitly because of feedback from the community and from the .NET SDK experts at Microsoft. Has we lock on v1.0 next week, all non-experimental APIs will not change.

I also want to share that we were able to republish all of the content in learn.Microsoft.com to reflect the latest in RC4. We expect few (if any) breaking changes from now and until V1.0 next week

@qihangnet
Copy link

@markwallace-microsoft @matthewbolanos Thank you very much for your timely reply. I respect your work and dedication. Regardless of weekdays or weekends, I am fortunate to witness the relentless efforts of your team.

I initiated and led the Chinese Semantic Kernel Technology Community, which currently has nearly 1000 members. Tomorrow, at the conference called “.NET Conf China 2023” in Beijing, we have arranged nearly ten lectures on AI and Semantic Kernel. However, the recent frequent disruptive changes have brought challenges to our community and triggered some negative reactions. We have advised everyone through articles and WeChat groups to update their projects after the official release of version 1.0, to avoid these changes causing significant negative impacts repeatedly. But, this also means that any information conveyed to the Semantic Kernel audience during this time may become outdated at any time, which is obviously not a pleasant experience for the participants. Based on these considerations and our doubts about the value of this PR, we are providing feedback to you.

I believe that perhaps the SK team should try a relatively conservative approach—marking the old methods as deprecated before introducing new methods, instead of immediately removing them. Progressive changes are usually more acceptable and understandable than disruptive changes.

Once again, thank you for taking the time to address my concerns amidst your busy schedule. If my words have been offensive in any way, please accept my sincere apology. I sincerely hope that the Semantic Kernel project can grow steadily and rapidly.

@qihangnet
Copy link

Oh,one more thing. Recently, the access level of some practical classes in the Semantic Kernel has been changed from public to internal. This change has affected some developers who used these classes and methods directly before. Due to this change, developers are now forced to extract and duplicate the necessary code from the source code of the Semantic Kernel repo, or create equivalent functionality on their own. If your schedule permits, I request that you evaluate this issue and consider possible solutions before the release of version 1.0. Thank you again.🤭

@stephentoub
Copy link
Member Author

the access level of some practical classes in the Semantic Kernel has been changed from public to internal. This change has affected some developers who used these classes and methods directly before.

It'd be helpful for future reference to know which types/members you're referring to and how they were being used / the scenario.

In the incubation of SK, there have been many experiments, many approaches tried, etc., and as is commonly the case with such efforts, that left a lot of things in an inconsistent state, many things that weren't necessarily still useful, etc. When preparing for a stable 1.0 release, one really needs to remove as much of that as possible, as once you're stable, you can effectively never remove anything, since you're signing up to avoid breaking changes. Since it's always possible to add later but effectively never possible to remove later, the general philosophy is to get rid of things that aren't deemed valuable, as they can be brought back later if proven otherwise.

Bryan-Roe pushed a commit to Bryan-Roe-ai/semantic-kernel that referenced this pull request Oct 6, 2024
@matthewbolanos, @markwallace-microsoft, entirely up to you whether you
want to merge or close this; fine with me either way, I'm just opening
this to make it easy to decide. Fixes
microsoft#4033.

This removes `KernelBuilder` and adds a static `CreateBuilder()` such
that instead of doing:
```C#
Kernel kernel = new KernelBuilder().Whatever().Build();
```
one would now do:
```C#
Kernel kernel = Kernel.CreateBuilder().Whatever().Build();
```
Everything else should be functionally the same.

---------

Co-authored-by: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.Net: Add Kernel.CreateBuilder()

8 participants