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

backend: (Part 1) Use DB config values for Deployments during runtime #918

Merged
merged 10 commits into from
Jan 23, 2025

Conversation

tianjing-li
Copy link
Collaborator

@tianjing-li tianjing-li commented Jan 21, 2025

I noticed while working on https://linear.app/cohereai/issue/TLK-2601/add-update-to-db-config-when-app-starts that in fact the DB values for deployment configs weren't being used at all during runtime. We were instead always defaulting to what was stored in our Pydantic settings.

This PR modifies core behaviour so we store actual keys that reflect what each deployment will fetch in the newly named
get_deployment_config_var method.

AI Description

This PR introduces a new DeploymentSettingsMixin class in src/backend/config/settings.py, which formats deployment configurations before saving them to the database. The mixin class includes a to_dict method that retrieves capitalized variable names from a list of strings and returns a new dictionary with these names as keys.

The PR also renames several functions and classes to include the word "instance" in their names, indicating a shift towards working with specific instances of deployments rather than general deployment concepts. This change is reflected in the following files:

  • src/backend/chat/custom/custom.py
  • src/backend/chat/custom/utils.py
  • src/backend/config/settings.py
  • src/backend/crud/deployment.py
  • src/backend/database_models/deployment.py
  • src/backend/model_deployments/azure.py
  • src/backend/model_deployments/bedrock.py
  • src/backend/model_deployments/cohere_platform.py
  • src/backend/model_deployments/sagemaker.py
  • src/backend/model_deployments/single_container.py
  • src/backend/model_deployments/utils.py
  • src/backend/routers/utils.py
  • src/backend/schemas/deployment.py
  • src/backend/services/deployment.py
  • src/backend/services/request_validators.py
  • src/backend/tests/unit/services/test_deployment.py

Additionally, the PR adds a new function, get_deployment_by_class_name, to src/backend/crud/deployment.py, which retrieves a deployment by its class name.

@codecov-commenter
Copy link

codecov-commenter commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 80.88235% with 13 lines in your changes missing coverage. Please review.

Project coverage is 77.26%. Comparing base (d966233) to head (bf071db).

Files with missing lines Patch % Lines
src/backend/chat/custom/utils.py 25.00% 3 Missing ⚠️
src/backend/model_deployments/azure.py 33.33% 2 Missing ⚠️
src/backend/model_deployments/single_container.py 33.33% 2 Missing ⚠️
src/backend/model_deployments/utils.py 77.77% 2 Missing ⚠️
src/backend/routers/utils.py 0.00% 2 Missing ⚠️
src/backend/config/settings.py 95.45% 1 Missing ⚠️
src/backend/crud/deployment.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #918      +/-   ##
==========================================
+ Coverage   77.23%   77.26%   +0.03%     
==========================================
  Files         281      281              
  Lines       11461    11485      +24     
==========================================
+ Hits         8852     8874      +22     
- Misses       2609     2611       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tianjing-li tianjing-li force-pushed the update-db-deployment-on-config-change branch from 534b48c to 0485124 Compare January 23, 2025 00:47
Copy link
Collaborator

@EugeneLightsOn EugeneLightsOn left a comment

Choose a reason for hiding this comment

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

Looks great.

@tianjing-li tianjing-li merged commit ec0c043 into main Jan 23, 2025
9 checks passed
@tianjing-li tianjing-li deleted the update-db-deployment-on-config-change branch January 23, 2025 14:54
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.

3 participants