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.

Sign up for GitHub

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

Jump to bottom

fix(sdk): best_of in completions was an int and should be a string #1031

Merged
merged 6 commits into from
Sep 16, 2024
Merged

fix(sdk): best_of in completions was an int and should be a string #1031

merged 6 commits into from
Sep 16, 2024

Conversation

lemonprogis
Copy link
Contributor

@lemonprogis lemonprogis commented Sep 13, 2024 •
edited
Loading

Description

the best_of in completion.proto hasn't been changed to a string like in others. Changed that and ran the code gen.

BREAKING CHANGES

N/A

CHANGES

Related Issue

Fixes #819

Checklist before merging

@netlify Netlify
Copy link

netlify bot commented Sep 13, 2024 •
edited
Loading

✅ Deploy Preview for leapfrogai-docs canceled.

Name Link
🔨 Latest commit 2e7286d
🔍 Latest deploy log https://app.netlify.com/sites/leapfrogai-docs/deploys/66e723965f218c00080ed531

@justinthelaw justinthelaw changed the title bug(api): best_of in completions was an int and should be a string fix(api): best_of in completions was an int and should be a string Sep 13, 2024
@justinthelaw justinthelaw added bug Something isn't working api labels Sep 13, 2024
gphorvath
gphorvath suggested changes Sep 13, 2024
View reviewed changes
Copy link

@gphorvath gphorvath left a comment

Choose a reason for hiding this comment

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

I would like to clear up the linting issues before merging. There is a lot of this that is a no-op diff in generated proto code, due to some difference in linting config. I am also curious why our linting workflow didn't catch this.

src/leapfrogai_sdk/audio/audio_pb2.pyi Show resolved Hide resolved
src/leapfrogai_sdk/audio/audio_pb2_grpc.py Show resolved Hide resolved
src/leapfrogai_sdk/proto/leapfrogai_sdk/completion/completion.proto Show resolved Hide resolved
@gphorvath gphorvath changed the title fix(api): best_of in completions was an int and should be a string fix(sdk): best_of in completions was an int and should be a string Sep 13, 2024
@lemonprogis lemonprogis marked this pull request as ready for review September 15, 2024 23:53
@lemonprogis lemonprogis requested a review from a team as a code owner September 15, 2024 23:53
jalling97
jalling97 approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@jalling97 jalling97 left a comment

Choose a reason for hiding this comment

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

Thanks for pushing these fixes!

@justinthelaw justinthelaw mentioned this pull request Sep 16, 2024
Closed
12 tasks
justinthelaw
justinthelaw approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@justinthelaw justinthelaw left a comment

Choose a reason for hiding this comment

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

@lemonprogis feel free to smash that merge button!

@lemonprogis lemonprogis merged commit 4e5c1b6 into main Sep 16, 2024
37 checks passed
@lemonprogis lemonprogis deleted the best-of-completion-to-string branch September 16, 2024 12:31
@github-actions github-actions bot mentioned this pull request Sep 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinthelaw justinthelaw justinthelaw approved these changes

@jalling97 jalling97 jalling97 approved these changes

@gphorvath gphorvath gphorvath approved these changes

Assignees
No one assigned
Labels
api bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug(api): Fix legacy completions API endpoint
4 participants
@lemonprogis @gphorvath @jalling97 @justinthelaw