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

bumped to v4 endpoints, removed feedback, and added /start return pos… #1

Merged
merged 2 commits into from
Aug 10, 2022

Conversation

erik-dunteman
Copy link
Contributor

Why

We've depricated parts of the backend in an attempt to simplify it of backward compatibility, therefore we're making fresh endpoints on a new major version to make sure that pre-serverless users of our backend have a stable 3.x.x release now that we're on 4.x.x

The API itself is not changing, so no need for users to rewrite their code. We're just deprecating old parts of the backend.

Testing

Tested end-to-end with recently deployed v4 endpoints on backend, everything works as expected.

const callID = await startAPI(apiKey, modelKey, modelInputs)
export async function runMain(apiKey: string, modelKey: string, modelInputs: object = {}): Promise<any>{
const startOut = await startAPI(apiKey, modelKey, modelInputs)
if (startOut["finished"] == true){
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you check what happens if "finished" doesn't exist? will this crash?

Copy link
Contributor

Choose a reason for hiding this comment

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

i.e assuming runMain is called for older sdks too can this ever not return the key properly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if /start doesn't return it for some reason, the SDK falls back on our /check endpoint which doesn't use this "finished" logic as a break condition.

Copy link
Contributor

@kylejmorris kylejmorris left a comment

Choose a reason for hiding this comment

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

one q about error handling but lgtm otherwise

@erik-dunteman
Copy link
Contributor Author

confirming this was deployed as 4.0.0

@erik-dunteman erik-dunteman merged commit 8ddc58d into main Aug 10, 2022
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