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(backend): implement brain-prompt link #831

Merged
merged 7 commits into from
Aug 3, 2023

Conversation

mamadoudicko
Copy link
Contributor

Description

  • Refactor Brains.py (split into Entity, Repo, and Service)
  • Add prompt_id to brain table
  • Add prompt CRU endpoints

@mamadoudicko mamadoudicko temporarily deployed to preview August 3, 2023 07:46 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Aug 3, 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 3, 2023 8:19am
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 3, 2023 8:19am

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

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

  1. The upload_file function could be improved by separating the file validation and the file processing into separate functions. This would make the code easier to read and maintain.

  2. The use of environment variables directly in the code can make it difficult to manage and test. Consider using a configuration management system to handle environment variables.

  3. The error messages returned by the function could be more descriptive. For example, instead of just saying 'User's brain will exceed maximum capacity with this upload', it could also include information about the current brain size and the size of the file being uploaded.

Example:

message = {
    "message": f"User's brain will exceed maximum capacity with this upload. Current brain size: {current_brain_size}, file size: {file_size}",
    "type": "error",
}

LOGAF Level 3 - /home/runner/work/quivr/quivr/backend/core/models/brains.py

  1. The openai_api_key field in the Brain class is a potential security risk. API keys should not be stored in plain text and should not be exposed. Consider using environment variables or a secure vault service to store sensitive information.

  2. The get_brain_users and delete_user_from_brain methods are marked as TODO to move to a new BrainService. It's a good practice to separate concerns and make the code more modular. Consider creating a new service class to handle these operations.

  3. The create method is using commons as a parameter in the class constructor. This could lead to potential issues if the commons object changes after the Brain object is created. Consider passing commons as a parameter to the methods that need it instead of storing it in the object.


LOGAF Level 3 - /home/runner/work/quivr/quivr/backend/core/repository/brain/set_as_default_brain_for_user.py

  1. Add type hints to the commons variable.

Example:

commons: Dict[str, Any] = common_dependencies()
  1. Add error handling for the case where get_user_default_brain or update operation fails.

📂🔒🔧


Powered by Code Review GPT

@mamadoudicko mamadoudicko changed the title feat: add prompt_id column to brain table feat(backend): implement brain-prompt link Aug 3, 2023
@mamadoudicko mamadoudicko temporarily deployed to preview August 3, 2023 07:56 — with GitHub Actions Inactive
BEGIN
IF NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'brains' AND column_name = 'prompt_id') THEN
-- Add prompt_id column
ALTER TABLE brains ADD COLUMN prompt_id UUID;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prompt_id can be null and shpild reference the table prompts' id column

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would akso need to add the new column in the brains in tables.sql

@mamadoudicko mamadoudicko merged commit 4ca6c66 into main Aug 3, 2023
6 of 7 checks passed
StanGirard pushed a commit that referenced this pull request Sep 12, 2023
* feat: add prompt_id field to brain

* feat(Prompt controller): update prompt routes

* feat: remove unused private prompts

* refactor: add BrainEntity and repo and service

* tests: partially type main Repository

* feat: add PromptStatusEnum enum

* feat: change delete prompt repository return type
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