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

Make get_api_key_from_environment use Result-Ok #787

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Make get_api_key_from_environment use Result-Ok #787

merged 3 commits into from
Jan 5, 2024

Conversation

rossdanlm
Copy link
Contributor

@rossdanlm rossdanlm commented Jan 5, 2024

Make get_api_key_from_environment use Result-Ok

Nice, cool tips from Jonathan

Also deleted test_config_util.py because it wasn't being referenced anywhere


Stack created with Sapling. Best reviewed with ReviewStack.

Rossdan Craig rossdan@lastmileai.dev added 3 commits January 5, 2024 17:17
Sometimes key is not defined or set, we need to do this to unblock #769

The functionality is still unchanged because the optional param `required` defaults to true

## Test Plan
Pass all automated tests (which it didn't before)
…call

See comments from #772 (comment)

## Test Plan
Run the Gemini cookbook to make sure it still works
Nice, cool tips from Jonathan

Also deleted `test_config_util.py` because it wasn't being referenced anywhere
Copy link
Contributor

@jonathanlastmileai jonathanlastmileai left a comment

Choose a reason for hiding this comment

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

accepting to unblock assuming this fixes a P0 bug and is tested. Let's talk about error handling and required vs. not as a follow up

rossdanlm added a commit that referenced this pull request Jan 5, 2024
…call (#786)

Set the Gemini api key in init instead of re-running every inference
call


See comments from
#772 (comment)

## Test Plan
Run the Gemini cookbook to make sure it still works

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/786).
* #787
* __->__ #786
* #772
@rossdanlm rossdanlm merged commit 26e26b2 into main Jan 5, 2024
1 check passed
@rossdanlm rossdanlm deleted the pr787 branch January 5, 2024 23:17
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.

2 participants