WIP Create PauseQueueJob

[sssoleileraaa]

Description

This is not ready to be reviewed but I'm opening a PR here to get an early preview of how I am intending to use a queue job to pause the queue. While working on this I realized that we would have to resume the main queue in order to process a logout job. Since we currently don't have a logout job, this isn't something that needs to be written right now, but we should be thinking about it.

Also, I don't see a need to persist anything on pause or on logout. We should update our local database to mark a reply or file as "pending" as soon as the jobs are created. Then when we log back in, the client can show anything that was in "pending" status as failed (or more intelligently pick up the file download jobs from where they left off).

We should discuss some of the details further as a team, but for now, I think it should be fine to move forward with a PauseQueueJob that has highest prioirty (until LogoutJob is created... but maybe we'll decide against it).

Resolves #443

Test Plan

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs, network (via the RPC service) traffic, or fine tuning of the graphical user interface, Qubes testing is required. Please check as applicable:

• I have tested these changes in Qubes
• I do not have a Qubes OS workstation (the reviewer will need to test these changes in Qubes)

[redshiftzero]

I like the PauseQueueJob approach, some thoughts inline

[redshiftzero]

btw before I forget - following up on our chat yesterday, I had some thoughts regarding how the LogoutJob should interact with this queue pausing functionality: the LogoutJob is really just a TokenInvalidationJob (we can even name it as such to be absolutely clear). The logout action in logic.py should just:

1. Submit job to queue to invalidate the token (we won't need ApiJobQueue.logout() anymore since the highest priority job other than pause will be invalidating the token).
2. Log out UI (so even if 1 fails due to network issues, the UI will still be in its logged out state).

This will get us the behavior that we want:

1. User has token A
2. Client loses network connectivity, queue will pause (aside but related: we should also have the queue pause if it ever hits an auth error because it means the user token expired or is invalidated)
3. User logs out. UI is in the logged out state immediately regardless of what the queue or network is doing.
4. User then logs back in and gets a new token B. Queue processing resumes. Their previous token will be invalidated before anything else happens as the logout/token invalidation job will be the highest priority job in the queue.

[sssoleileraaa]

Ah, yes, I like how you broke logout down into the two pieces: TokenInvalidationJob (the api piece) and signing the user out in the client.

3. User logs out. UI is in the logged out state immediately regardless of what the queue or network is doing.

This makes sense to me. Just to expand on this... Once the network-health-check functionality is implemented, the client will be able to auto-resume a queue after network issues resolve. This means that we could allow the queues to resume even when the user is logged out. However, the TokenInvalidationJob will always be the first job to be processed after a logout, so I think this means only PauseQueueJob will get processed when signed out after TokenInvalidationJob is processed. Is that your understanding too?

2. Client loses network connectivity, queue will pause (aside but related: we should also have the queue pause if it ever hits an auth error because it means the user token expired or is invalidated)

Created an Issue for this aside: #517

[redshiftzero]

what about priority 10 and not 11? it seems like this is already the current behavior since the order_number for a pause job is 1 meaning that the pause will always run before other jobs with priority 11

[sssoleileraaa]

I think it would be cleaner if we use a priority of 10 for InvalidTokenJob and keep this at 11 rather than relying on order_number. I think order_number makes more sense for things like SendReplyJobs because they are actually the same type of job where order matters.

[redshiftzero]

heads up if PauseQueueJob doesn't inherit from ApiJob or implement __lt__, an exception will arise if duplicate pause jobs ever enter the queue - what do you think about inheriting from ApiJob?

[sssoleileraaa]

Ooooh, hmmmmm

I think as the client code base continues to grow, we will have more jobs that are not ApiJobs, but for now I like that it's a quick solution and I'll follow-up with filing an issue.

[sssoleileraaa]

Actually, I think it's cleaner to keep PauseQueueJob implemented like:

class PauseQueueJob(QObject):
    def __init__(self):
        super().__init__()
        self.order_number = None

And then have the enqueue function (1) add it to both of the queues after receiving a network timeout error and (2) emit a signal to the controller so it can update the error status bar:

    def enqueue(self, job: ApiJob) -> None:
        ...
        if isinstance(job, PauseQueueJob):
            self.main_queue.add_job(priority, job)
            self.download_file_queue.add_job(priority, job)
            self.paused.emit()
        ...

[sssoleileraaa]

Closing to open a new PR with a better description of the upcoming changes after receiving early feedback (thanks redshift!)