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: update backend tests #975

Merged
merged 2 commits into from
Aug 18, 2023
Merged

fix: update backend tests #975

merged 2 commits into from
Aug 18, 2023

Conversation

mamadoudicko
Copy link
Contributor

Description

  • Fix backend tests

@mamadoudicko mamadoudicko temporarily deployed to preview August 18, 2023 09:04 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Aug 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 18, 2023 10:34am
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 18, 2023 10:34am

@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2023

Risk Level 2 - /home/runner/work/quivr/quivr/backend/core/vectorstore/supabase.py

The addition of the embedding parameter in the __init__ method of CustomSupabaseVectorStore could potentially break existing code if not all calls to this method have been updated. Ensure that all calls to this method include the new parameter.


Risk Level 3 - /home/runner/work/quivr/quivr/backend/core/routes/chat_routes.py

  1. The check_user_limit function now reads the MAX_REQUESTS_NUMBER environment variable. This is a good change as it allows the maximum number of requests to be configured without changing the code. However, it's important to ensure that this environment variable is set, otherwise the code will default to 1000.

  2. The create_stream_question_handler function has been significantly modified. The changes seem to be aimed at making the function more flexible and able to handle different models and parameters. However, the changes are quite complex and could potentially introduce bugs. It's important to thoroughly test this function to ensure it works as expected.


Risk Level 3 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/components/SettingsTab/hooks/useSettingsTab.ts

The changes in this file are moderate risk. The changes are mainly related to form submission and event handling. However, there are a few potential issues:

  1. The use of void before handleSubmit(true). This is not a common practice and can lead to confusion. It would be better to handle the promise returned by handleSubmit(true) properly. For example:
handleSubmit(true).then(() => {}).catch((error) => console.error(error));

This way, any errors that occur during the execution of handleSubmit(true) will be caught and handled properly.

  1. The use of setTimeout to set the value of model. This can lead to race conditions if the value of model is needed before the timeout has completed. It would be better to set the value of model immediately, or use a state management library that supports asynchronous actions.

🔍🐛⏱️


Powered by Code Review GPT

@mamadoudicko mamadoudicko temporarily deployed to preview August 18, 2023 10:31 — with GitHub Actions Inactive
@mamadoudicko mamadoudicko merged commit c746eb1 into main Aug 18, 2023
StanGirard added a commit that referenced this pull request Aug 19, 2023
StanGirard added a commit that referenced this pull request Aug 19, 2023
StanGirard pushed a commit that referenced this pull request Sep 12, 2023
* fix: update backend tests

* fix(pytest): update types
StanGirard added a commit that referenced this pull request Sep 12, 2023
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.

2 participants