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

fix: Allow azure assistants chats to be deleted #3893

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

s-diez
Copy link
Contributor

@s-diez s-diez commented Sep 2, 2024

Summary

Deletion of an assitants chat with azureAssistants through the left sidebar will crash the application with the following error.

error: There was an uncaught error: Assistants API key not provided. Please provide it again.

This happen as the assistant chat is detected as a conversation with an assitant endpoint with a dedicated delete API call.
But there exists no detection which assitant endpoint (Azure or OpenAI) is used. Therefore if Azure is used and no OpenAI API key is configured the application still tries to delete the conversation through an API call with OpenAI and then crash.

I added an optional endpoint property to the delete mutation to pass along the desired endpoint.

Change Type

Please delete any irrelevant options.

  • Bug fix (non-breaking change which fixes an issue)

Testing

Build the latest docker image from main. Start the application with an azure assistant endpoint and plugin configured. Do not provide a OpenAI API key. Start a chat with the assistant after the response delete the conversation through the left sidebar. Observe the application crashing.

Apply suggested changes and repeat the previous steps. Observe no crashes, errors or warnings. The chat is is deleted. Start a normal chat without assistant and delete it aswell. Again no warnings etc.

Checklist

Please delete any irrelevant options.

  • My code adheres to this project's style guidelines

  • I have performed a self-review of my own code

  • My changes do not introduce new warnings

  • Any changes dependent on mine have been merged and published in downstream modules.

  • Local unit tests pass with my changes

npm run test:api did not pass on the upstream main as I seem to miss some configuration. Therefore I could not validate those tests running

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant