semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard

Adding option to use private PAT token on GitHub Q&A sample

Open mahomedalid opened this issue 2 years ago • 1 comments

Motivation and Context

  1. Why is this change required? What problem does it solve? What scenario does it contribute to?

Most integrations of OpenAPI and Semantic Kernel would have the motivation to integrate it with private information or non-public data. GitHub Q&A is an amazing sample, and be able to try it with private repositories it is a very straightforward way to solve scenarios where customers and users want to try it with private data.

Fixes #444

Description

  • Added a new Input in the React App, this is a password type of input. I decided to not include this pat token as part of the verification if it is a different repo.
  • In the GitHub skill I added the PatToken parameter using the existing context variables pattern. I saw this as straightforward since the PatToken may be used for other GitHub skill features.
  • WebFileDownloadSkill is modified to receive any additional headers to the request. I see that WebFileDownloadSkill could have had 3 ways to modify the headers of the request: 1) modify the constructor and class to receive and/or make HttpClient public, 2) Add the headers as part of the context variables received, 3) Override the download methods available to have one that specifically receive custom headers. I decided to use 3 to have the minimal impact in existing code (and tests), although I could see 2 could also be a good approach. Then the GitHub skill detects that an authorization header is necessary, and calls the specific overrided method.

Pending

  • Add Unit and/or integration tests.

Contribution Checklist

  • [X] The code builds clean without any errors or warnings
  • [X] The PR follows SK Contribution Guidelines (https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
  • [X] The code follows the .NET coding conventions (https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions) verified with dotnet format
  • [ ] All unit tests pass, and I have added new tests where possible
  • [ ] I didn't break anyone :smile:

mahomedalid avatar Apr 28 '23 02:04 mahomedalid

@craigomatic can you take a look at this PR?

evchaki avatar May 25 '23 20:05 evchaki

@adrianwyatt @craigomatic I saw a change made from the call to an older github api to the latest one. The latest one changes two things: now it redirects to a temporary (5min) private link (302), the redirect request will also need the authorization headers, and it requires extra headers (X-GitHub-Api-Version, User-Agent, Accept), which were not there. Updated, and should be back to working state now.

mahomedalid avatar Jun 21 '23 19:06 mahomedalid