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 Samples Restructuring - Phase 2 #6005

Merged

Conversation

RogerBarreto
Copy link
Member

@RogerBarreto RogerBarreto commented Apr 25, 2024

Phase 2 - Sample Restructuring

Resolves #3978
Resolves #5976

  • Removal of Examples numbering
  • Decomposing KernelSyntaxExamples into meaningful Feature base folders within Concepts.
  • Update current sample projects to use sample centric InternalUtilities reducing code repetition
  • Update SDK projects type to be correctly idenfified as xUnit Test projects
  • Update projects to be implicit using friendly, reducing considerably the extra lines of code for every sample.
  • Decomposing Concepts\AgentSyntax into Concepts\Agents
  • Decomposing Concepts\AgentSyntax\GettingStarted into a new root dedicated GettingStartedWithAgents

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.

Added a few minor comments that should be quick to fix

@RogerBarreto RogerBarreto added this pull request to the merge queue Apr 29, 2024
Merged via the queue into microsoft:main with commit c545c7d Apr 29, 2024
15 checks passed
@RogerBarreto RogerBarreto deleted the features/samples-restructure-phase2 branch April 29, 2024 15:55
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
## Phase 2 - Sample Restructuring

- Removal of Examples numbering 
- Decomposing `KernelSyntaxExamples` into meaningful Feature base
folders within `Concepts`.
- Update current sample projects to use `sample` centric
`InternalUtilities` reducing code repetition
- Update SDK projects type to be correctly idenfified as xUnit Test
projects
- Update projects to be implicit using friendly, reducing considerably
the extra lines of code for every sample.
- Decomposing `Concepts\AgentSyntax` into `Concepts\Agents`
- Decomposing `Concepts\AgentSyntax\GettingStarted` into a new root
dedicated `GettingStartedWithAgents`
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
## Phase 2 - Sample Restructuring

- Removal of Examples numbering 
- Decomposing `KernelSyntaxExamples` into meaningful Feature base
folders within `Concepts`.
- Update current sample projects to use `sample` centric
`InternalUtilities` reducing code repetition
- Update SDK projects type to be correctly idenfified as xUnit Test
projects
- Update projects to be implicit using friendly, reducing considerably
the extra lines of code for every sample.
- Decomposing `Concepts\AgentSyntax` into `Concepts\Agents`
- Decomposing `Concepts\AgentSyntax\GettingStarted` into a new root
dedicated `GettingStartedWithAgents`
LudoCorporateShark pushed a commit to LudoCorporateShark/semantic-kernel that referenced this pull request Aug 25, 2024
## Phase 2 - Sample Restructuring

- Removal of Examples numbering 
- Decomposing `KernelSyntaxExamples` into meaningful Feature base
folders within `Concepts`.
- Update current sample projects to use `sample` centric
`InternalUtilities` reducing code repetition
- Update SDK projects type to be correctly idenfified as xUnit Test
projects
- Update projects to be implicit using friendly, reducing considerably
the extra lines of code for every sample.
- Decomposing `Concepts\AgentSyntax` into `Concepts\Agents`
- Decomposing `Concepts\AgentSyntax\GettingStarted` into a new root
dedicated `GettingStartedWithAgents`
baywet added a commit to baywet/semantic-kernel that referenced this pull request Oct 25, 2024
github-merge-queue bot pushed a commit that referenced this pull request Nov 15, 2024
…r fixes. (#9436)

fixes #6614

### Motivation and Context

- Adds support for Microsoft Plugins Manifest to semantic kernel in
dotnet.
- Fixes performance bottleneck for API Manifest loading.
- Fixes broken integration tests for API manifest loading.
- Adds an OpenAPI operation id normalization visitor to replace `.` by
`_` so function names are valid.
- Fixes performance bottleneck in OpenAPI operation loading.
- Fixes experimental ID for API manifest from SKEXP0043 to SKEXP0040
after the renumbering
- Upgrades OAI.net and ApiManifest references.


### 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: -->

- [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
- [x] I didn't break anyone 😄

On the unit tests: I'd like guidance on where to add unit tests for:

- API Manifest loading (wasn't done at the time)
- Microsoft Manifest loading.
- Operation Id Normalization Visitor

### Why so many files?

- `./kiota`: all generated files, it contains kiota workspace
configuration, which comes with copies of the OAS descriptions, etc… You
don’t need to review it manually. It is useful because it allows us to
automatically generate/refresh the integration tests plugins. If you
feel like it's adding too much noise, we can remove those, we'll loose
the ability to refresh the plugins definitions. https://aka.ms/kiota
- `Samples/concepts`: restored the api manifest for astronomy API to fix
the API manifest integration test. Added sample Microsoft Manifest for
the new integration tests. Those are automatically generated via kiota
and can be automatically refreshed later.
- `Src/Functions`: Microsoft manifests implementation, API manifest
fixes.


### How to run the local tests

Create the following JSON file
`D:\github\semantic-kernel\dotnet\samples\Concepts\bin\Debug\net8.0\appsettings.Development.json`
(for some reason given how the project is setup this file is not being
copied automatically. I didn't to touch any of the project setup out of
fear of breaking other things)

```json
{
  "MSGraph": {
    "ClientId": "clientId",
    "TenantId": "tenantId",
    "Scopes": [
      "Calendars.Read",
      "Contacts.Read",
      "Files.Read.All",
      "Mail.Read",
      "User.Read"
    ],
    "RedirectUri": "http://localhost"
  }
}
```

Replace the clientId and TenantId by your own value.
To create the application registration, go to
https://aad.portal.azure.com, create a new application registration, new
public client (add the redirect URI). In API access, add the listed
Microsoft Graph delegated scopes. Grant consent after adding the scopes.
During the first run, your browser will open to get the token.

### File paths and copies

Like for the development settings, the project is NOT copying the sample
plugin files for some reason. This is why the loading of the files in
the integration tests looks at source files directly with `../../../`
path segments. Happy to review that upon guidance.

### License for Astrology plugins

The description is under the Apache license. I added the plugin (API and
Microsoft) to restore the integration test for the former and mirror the
setup to the latter. In the case of API plugins, we're only referring to
it, so having a plugin is fine. In case of the Microsoft plugin, we have
a full copy under the kiota configuration directory, and a sliced down
version (derived work) in the example plugin. The value of this API is
that it allows us to test scenarios outside of Microsoft Graph. But if
the license is a challenge, we can remove those before merging.
@RogerBarreto to provide more context on why those were deleted at the
first place in #6005

### Why so many commits?

Incremental work during the implementation, plus regular merges from
main to make sure everything was current and we wouldn't end up with
conflicts, etc... Happy to rebase and squash once the initial reviews
are through.

---------

Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Mustafa Zengin <muzengin@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation kernel.core kernel Issues or pull requests impacting the core kernel memory .NET Issue or Pull requests regarding .NET code
Projects
Archived in project
3 participants