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(question): fixed with user_settings #1349

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Conversation

StanGirard
Copy link
Collaborator

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Checklist before requesting a review

Please delete options that are not relevant.

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented hard-to-understand areas
  • I have ideally added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged

Screenshots (if appropriate):

@StanGirard StanGirard temporarily deployed to preview October 6, 2023 18:32 — with GitHub Actions Inactive
@dosubot dosubot bot added the area: backend Related to backend functionality or under the /backend directory label Oct 6, 2023
@vercel
Copy link

vercel bot commented Oct 6, 2023

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

Name Status Preview Updated (UTC)
docs 🔄 Building (Inspect) Visit Preview Oct 6, 2023 6:32pm
quivr-strapi 🔄 Building (Inspect) Visit Preview Oct 6, 2023 6:32pm
quivrapp 🔄 Building (Inspect) Visit Preview Oct 6, 2023 6:32pm

@StanGirard StanGirard merged commit b5c01ef into main Oct 6, 2023
5 of 9 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

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

  1. The delete_chat_from_db function catches all exceptions and simply prints them. This could make debugging difficult in the future as it swallows all errors. Consider logging the error message and re-throwing the exception or handling specific exceptions.
try:
    supabase_db.delete_chat_history(chat_id)
except Exception as e:
    logger.error(e)
    raise
  1. The check_user_requests_limit function has a pass statement in the else clause. If there is no code to execute, consider removing the else clause altogether.

  2. The is_model_ok variable is defined twice in the create_question_handler function. This could lead to confusion and potential bugs. Consider refactoring the code to define it only once.

  3. The create_question_handler and create_stream_question_handler functions have a lot of duplicate code. Consider refactoring to a common function to improve maintainability and reduce potential for bugs.

  4. The print statements in the create_stream_question_handler function could be replaced with proper logging for better traceability in a production environment.

logger.info(\"streaming\")

🐛🔄🔍


Powered by Code Review GPT

@sentry-io
Copy link

sentry-io bot commented Oct 9, 2023

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ AttributeError: 'dict' object has no attribute 'models' /chat/{chat_id}/question View Issue

Did you find this useful? React with a 👍 or 👎

gozineb pushed a commit that referenced this pull request Oct 9, 2023
🤖 I have created a release *beep* *boop*
---


## 0.0.89 (2023-10-09)

## What's Changed
* feat: enable CSP in all environments (local/preview/prod) by
@matthieujacq in #1334
* feat: enhance user page UI by @nguernse in
#1319
* feat: update onboarding steps by @mamadoudicko in
#1337
* feat: add onboarding_a column to onboarding table by @mamadoudicko in
#1340
* fix(question): fixed with user_settings by @StanGirard in
#1349
* FIX tables.sql - missing ; breaks SQL queries. by @stanrb in
#1348
* feat: ⚙️🐞 configure debugger for the backend by @matthieujacq in
#1345
* test: add chat e2e tests by @mamadoudicko in
#1344
* feat: configure CSP for self-hosting and multiple ports in dev mode by
@matthieujacq in #1364

## New Contributors
* @stanrb made their first contribution in
#1348

**Full Changelog**:
v0.0.88...v0.0.89

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
coolCatalyst added a commit to coolCatalyst/quivr that referenced this pull request Jun 1, 2024
🤖 I have created a release *beep* *boop*
---


## 0.0.89 (2023-10-09)

## What's Changed
* feat: enable CSP in all environments (local/preview/prod) by
@matthieujacq in QuivrHQ/quivr#1334
* feat: enhance user page UI by @nguernse in
QuivrHQ/quivr#1319
* feat: update onboarding steps by @mamadoudicko in
QuivrHQ/quivr#1337
* feat: add onboarding_a column to onboarding table by @mamadoudicko in
QuivrHQ/quivr#1340
* fix(question): fixed with user_settings by @StanGirard in
QuivrHQ/quivr#1349
* FIX tables.sql - missing ; breaks SQL queries. by @stanrb in
QuivrHQ/quivr#1348
* feat: ⚙️🐞 configure debugger for the backend by @matthieujacq in
QuivrHQ/quivr#1345
* test: add chat e2e tests by @mamadoudicko in
QuivrHQ/quivr#1344
* feat: configure CSP for self-hosting and multiple ports in dev mode by
@matthieujacq in QuivrHQ/quivr#1364

## New Contributors
* @stanrb made their first contribution in
QuivrHQ/quivr#1348

**Full Changelog**:
QuivrHQ/quivr@v0.0.88...v0.0.89

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: backend Related to backend functionality or under the /backend directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant