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.

Sign up for GitHub

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

Jump to bottom

Update ImportDocument tool to target hosted API #231

Closed
crickman opened this issue Aug 21, 2023 · 0 comments · Fixed by #239
Closed

Update ImportDocument tool to target hosted API #231

crickman opened this issue Aug 21, 2023 · 0 comments · Fixed by #239
Assignees
@gitri-ms
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request memory Issues or questions related to memory sk team issue

Comments

@crickman
Copy link
Contributor

  1. Include required request headers to target hosted API (not just local).
  2. Update documentation to clearly address configuration for hosted deployment.

A user with a hosted chat-copilot application should be able to use the importdocument tool to upload documents into chat or global memory.

See customer issue: #229
@crickman crickman added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request memory Issues or questions related to memory labels Aug 21, 2023
@crickman crickman mentioned this issue Aug 21, 2023
Closed
@gitri-ms gitri-ms self-assigned this Aug 21, 2023
@gitri-ms gitri-ms mentioned this issue Aug 23, 2023
Merged
4 tasks
github-merge-queue bot pushed a commit that referenced this issue Aug 23, 2023
@gitri-ms
Update import document tool to new auth model (#239)
3838fca 
### Motivation and Context
The recent changes to the authentication model in Chat Copilot
effiectively broke the import document tool. This change fixes it, by
enabling users to:
- Use the tool with an unauthenticated local instance of Chat Copilot
without signing in
- Use the tool with an Azure-deployed instance of Chat Copilot with
Azure AD authentication enabled

Fixes #212, fixes #231

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

- Adds code paths to authenticate user (or not) depending on
configuration
- Removes user name/info from document import form
- Updates README with instructions for setting up the app registration
for import document tool and configuring appsettings.json

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible -
N/A, but both supported scenarios have been tested
- [x] I didn't break anyone 😄
teamleader-dev pushed a commit to vlink-group/chat-copilot that referenced this issue Oct 7, 2024
@gitri-ms
Update import document tool to new auth model (microsoft#239)
a16f5e5 
### Motivation and Context
The recent changes to the authentication model in Chat Copilot
effiectively broke the import document tool. This change fixes it, by
enabling users to:
- Use the tool with an unauthenticated local instance of Chat Copilot
without signing in
- Use the tool with an Azure-deployed instance of Chat Copilot with
Azure AD authentication enabled

Fixes microsoft#212, fixes microsoft#231

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

- Adds code paths to authenticate user (or not) depending on
configuration
- Removes user name/info from document import form
- Updates README with instructions for setting up the app registration
for import document tool and configuring appsettings.json

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible -
N/A, but both supported scenarios have been tested
- [x] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gitri-ms gitri-ms

Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request memory Issues or questions related to memory sk team issue
Projects
No open projects
Apps & Services Semantic Kernel
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update import document tool to new auth model gitri-ms/chat-copilot
2 participants
@gitri-ms @crickman