qubes: Add client-side timeouts #547
Conversation
apyrgio
added
enhancement
There was a problem hiding this comment.
This is an initial review. Next week I'll do a more thorough one.
Also, the approach here seems to be a constant timeout for the whole document calculated at the beginning. I was thinking more of a per-page timeout. Is the intention to make this a per-page timeout once we actually stream the pages? (currently we only send them all in bulk when all is finished because we are blocked on #443)
Yeap, ideally we should make it a per-page timeout, but we need to get rid of the server-side qubes wrapper to do that. |
|
Ouch, there are lots of errors in this PR that I didn't see in my local environment. I'll send fixes for them soon as well. |
Add a simple stopwatch implementation to track the elapsed time since an event, or the remaining time until a timeout.
Add a function that can read data from non-blocking fds, which we will used later on to read from standard streams with a timeout.
Factor out the logic behind the calculate_timeout() method, used in Dangerzone conversions, so that isolation providers can call it directly.
apyrgio
force-pushed
the
446-client-timeouts
branch
from
1b7b9eb
to
6e05e3b
Compare
Extend the client-side capabilities of the Qubes isolation provider, by adding client-side timeout logic. This implementation brings the same logic that we used server-side to the client, by taking into account the original file size and the number of pages that the server returns. Since the code does not have the exact same insight as the server has, the calculated timeouts are in two places: 1. The timeout for getting the number of pages. This timeout takes into account: * the disposable qube startup time, and * the time it takes to convert a file type to PDF 2. The total timeout for converting the PDF into pixels, in the same way that we do it on the server-side. Besides these changes, we also ensure that partial reads (e.g., due to EOF) are detected (see exact=... argument) Some things that are not resolved in this commit are: * We have both client-side and server-side timeouts for the first phase of the conversion. Once containers can stream data back to the application (see #443), these server-side timeouts can be removed. * We do not show a proper error message when a timeout occurs. This will be part of the error handling PR (see #430) Fixes #446 Refs #443 Refs #430
apyrgio
force-pushed
the
446-client-timeouts
branch
from
6e05e3b
to
99dd5f5
Compare
Extend the client-side capabilities of the Qubes isolation provider, by
adding client-side timeout logic.
This implementation brings the same logic that we used server-side to
the client, by taking into account the original file size and the number
of pages that the server returns.
Since the code does not have the exact same insight as the server has,
the calculated timeouts are in two places:
account:
that we do it on the server-side.
Besides these changes, we also ensure that partial reads (e.g., due to
EOF) are detected (see exact=... argument)
Some things that are not resolved in this PR are:
of the conversion. Once containers can stream data back to the
application (see Containers: have progress streamed instead of via mounted volumes (and deprecate doc_to_pixels_qubes_wrapper.py) #443), these server-side timeouts can be removed.
be part of the error handling PR (see Qubes: Error handling #430)
Fixes #446
Refs #443
Refs #430