-
Notifications
You must be signed in to change notification settings - Fork 4
Fix too early task removal and startup conditions #8
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
base: server
Are you sure you want to change the base?
Conversation
mpulukkinen
commented
Dec 8, 2025
- Remove the task only after completed and client polls the result
- Remove the exit's when there's parameters that are not needed for server
- No longer exits if non-required parameters are missing
- Only erase when client polls the result and task is failed or done
|
The reason I was erasing the data (which is the preview or final image) after sending the result was to avoid spamming the same image data multiple times over the network, so the client can assume that if it receives no data, the last image received is the lastest. Maybe there's another way to achieve the same without dropping the data. Was the current implementation causing issues? |
|
Yeah, at least at first, because my app (usually) polls the result. So I was getting null result back and was very confused. Also, I was not getting the current progress. The task gives out the step and max steps, so I was using it to show progress. I wouldn't be so concerned about the preview data being transferred... Maybe the preview data could be cleared every time the result is sent? That way we could have best of the both worlds? :) |
Isn't that what the current implementation already does? |
|
I think removing the unnecessary parameters is a good change, not convinced yet about the other thing |
Yeah but it cleared all the results for the task (id) and next poll would give just null response to the client, which confused me a lot. In general, the servers should not send null results since the clients can't really know what's going on... |
|
Also, I'd like to add that from the client side point of view, it is difficult to know what's the actual situation if server sends null response: did the server crash? Did I just send wrong task id? Was the endpoint unreachable etc... |
|
@mpulukkinen if the server responds at all it means it hasn't crashed. But fair enough. Maybe I'll refactor the API a bit so that the server doesn't send any additional data per default, just a flag that is set to true if a new preview is available, and another enpoint to grab the lastest preview/result |
|
I'm not sure if the refactoring is needed, even with this change. I think the current API is good, because it's quite simple. I've worked with lots of other genAi apis, it's pretty common that the result fields are empty when polling, until there's some results to be fetched. Same idea could work here: preview is non-null if there's new preview. And result is send once. Well, now that I think of it, client side could botch the fetch and the result would be lost forever :D |