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

feat(llm): removing all llms to prepare for genoss #804

Merged
merged 4 commits into from
Jul 31, 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 July 31, 2023 10:29 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Jul 31, 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 Jul 31, 2023 1:56pm
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2023 1:56pm

@github-actions
Copy link
Contributor

github-actions bot commented Jul 31, 2023

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

  1. The delete_chat_from_db function is catching all exceptions and simply printing them. This is not a good practice as it can make debugging difficult. Instead, consider logging the exceptions and re-throwing them so they can be handled upstream.
try:
    commons["supabase"].table("chat_history").delete().match({"chat_id": chat_id}).execute()
except Exception as e:
    logger.error(e)
    raise
  1. The check_user_limit function is using os.getenv to get the MAX_REQUESTS_NUMBER environment variable. This is not a good practice as it can lead to unexpected behavior if the environment variable is not set. Instead, consider using a configuration file or a dedicated configuration management system.

  2. The get_chat_details and fetch_user_stats functions are directly interacting with the database. This is not a good practice as it can make the code difficult to test and maintain. Instead, consider using a data access layer or an ORM.


LOGAF Level 3 - /home/runner/work/quivr/quivr/backend/core/utils/vectors.py

  1. The create_vector function is catching all exceptions and simply logging them. This is not a good practice as it can make debugging difficult. Instead, consider re-throwing the exceptions so they can be handled upstream.
try:
    sids = self.commons["documents_vector_store"].add_documents([doc])
    if sids and len(sids) > 0:
        return sids
except Exception as e:
    logger.error(f"Error creating vector for document {e}")
    raise
  1. The process_batch function is directly interacting with the database. This is not a good practice as it can make the code difficult to test and maintain. Instead, consider using a data access layer or an ORM.

🚫🐞🗄️


Powered by Code Review GPT

@StanGirard StanGirard temporarily deployed to preview July 31, 2023 13:55 — with GitHub Actions Inactive
@StanGirard StanGirard changed the title Feat/genoss feat(llm): removing all llms to prepare for genoss Jul 31, 2023
@StanGirard StanGirard marked this pull request as ready for review July 31, 2023 13:55
@StanGirard StanGirard merged commit db40f3c into main Jul 31, 2023
6 of 8 checks passed
StanGirard added a commit that referenced this pull request Sep 12, 2023
* feat(routes): removed all except openai

* feat(deadcode): removed some deadcode and summarization feature that wasn't used

* feat(streaming): removed privateGPT from it

* chore(requirements): increased version
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.

None yet

2 participants