Skip to content

[backend] api_key not set for class methods #191

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

Merged
merged 2 commits into from
Jun 11, 2024
Merged

Conversation

scott-cohere
Copy link
Contributor

@scott-cohere scott-cohere commented Jun 10, 2024

For CohereDeployment when we call the class method list_model() we don't actually have the API key set as a class attribute, rather it is set on __init__ as a instance attribute. To fix this we can just set the API key at the class attribute level and both class and instance methods can both access it.

The other deployment class definitions don't have this problem. They just have a list of the model as a class attribute.

Tested locally
image
image

AI Description

This pull request updates the CohereDeployment class in cohere_platform.py to use the get_model_config_var function to retrieve the API key from the environment variable COHERE_API_KEY_ENV_VAR.

File Changes

  • src/backend/model_deployments/cohere_platform.py

Code Changes

  • The api_key attribute of the CohereDeployment class is now assigned using the get_model_config_var function, passing in COHERE_API_KEY_ENV_VAR as an argument.
  • Removed the assignment of self.api_key in the __init__ method, as it is now assigned in the class definition.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@bcaab53). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #191   +/-   ##
=======================================
  Coverage        ?   87.51%           
=======================================
  Files           ?      130           
  Lines           ?     4390           
  Branches        ?        0           
=======================================
  Hits            ?     3842           
  Misses          ?      548           
  Partials        ?        0           

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

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 good to me

@scott-cohere scott-cohere merged commit c5266b2 into main Jun 11, 2024
2 checks passed
@scott-cohere scott-cohere deleted the scott/env-api-key-fix branch June 11, 2024 14:18
ClaytonSmith pushed a commit to MiniMap-ai/cohere-toolkit that referenced this pull request Jun 11, 2024
ClaytonSmith added a commit to MiniMap-ai/cohere-toolkit that referenced this pull request Jun 17, 2024
* [coral-web] rename schema fields and update openapi client (cohere-ai#189)

* update

* update client

* Add use agents view env variable.  (cohere-ai#192)

* Change

* Format

* [backend] api_key not set for class methods (cohere-ai#191)

* fix

* easier fix

* perf: downgrade rehype-highlight and generate once blob url (cohere-ai#193)

* perf: downgrade rehype-highlight and generate once blob url

* fix: replace code blocks to iframe in conversation history

* refactor: run format

* refactor: pr feedback

* Add OIDC auth strategy, add Blacklist scaffolding (model/schema/crud) (cohere-ai#190)

* add oidc, blacklist logic

* Add auth

* Add logic for GET /auth

* Mods

* fix: html preview with citations (cohere-ai#200)

* [backend] adding tools for agents, filter tools by agent_id (cohere-ai#204)

* changes

* lint

* filter tools by agent

* lint

* update tests

* lint and squash migrations

* fix alembic migration err

* lint

* merge (#3)


* [backend] make deployment field optional in API and DB (cohere-ai#213)

---------

Co-authored-by: Tianjing Li <[email protected]>
Co-authored-by: Tomeu <[email protected]>
Co-authored-by: Scott <[email protected]>
Co-authored-by: Beatrix De Wilde <[email protected]>

* pulling from main

* cleaned backend docker image push

* syncing with main (conflicts)

---------

Co-authored-by: Scott <[email protected]>
Co-authored-by: Beatrix De Wilde <[email protected]>
Co-authored-by: Tomeu <[email protected]>
Co-authored-by: Tianjing Li <[email protected]>
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.

6 participants