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

Bug fixes for the wait option in replicate.run #315

Merged
merged 5 commits into from
Oct 1, 2024
Merged

Bug fixes for the wait option in replicate.run #315

merged 5 commits into from
Oct 1, 2024

Conversation

aron
Copy link
Contributor

@aron aron commented Oct 1, 2024

There were a couple of small bugs in the current implementation:

  1. We would pass non-boolean, non-integer values through to predictions.create when it was an object with an interval, resulting in the blocking mode being used accidentally.
  2. We would pass the boolean/integer values through to wait which would create a runtime error when the wait function expects an object.
  3. We continued to poll for the prediction despite the blocking response returning the output data.

This PR addresses these three issues by checking if the run should be blocking and passing the correct arguments in the correct places. We also assume that if the returned prediction is not in starting state then it is completed. This isn't ideal but works for the moment.

Lastly, in the case where the blocking request times out the client will fall back to polling at the default interval.

mattt and others added 2 commits October 1, 2024 13:52
This commit fixes the use of the `run.wait` parameter to only pass it to
the `prediction.create` function if it's a number or integer. We now
check to see if the status of the object returned has moved out of a
starting state.
@aron aron requested a review from mattt October 1, 2024 21:13
index.d.ts Outdated
@@ -162,7 +162,7 @@ declare module "replicate" {
signal?: AbortSignal;
},
progress?: (prediction: Prediction) => void
): Promise<object>;
): Promise<unknown>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably a breaking change... maybe it's best left for another time.

@aron aron merged commit c1a12b0 into main Oct 1, 2024
7 checks passed
@aron aron deleted the fix-wait branch October 1, 2024 22:44
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