Skip to content

Commit

Permalink
fix: Fix API pause_all and resume_all methods
Browse files Browse the repository at this point in the history
These methods were previously making use of API.pause and API.resume
respectively, with the list of all downloads. It was problematic for a
few reasons:

- trying to pause an already paused download, or to resume an already
  active one does not make sense. Since we paused or resumed all
  downloads at once, the result of pause_all or resume_all would be
  False if even one download was already paused/active.
- second, pausing downloads one after the other, non-atomically, was
  was giving time to aria2 to start waiting downloads, which was not
  a desired effect. Until now, the pausing/starting was always done
  in such an order that all downloads were paused at the end, but I
  cannot ensure it behaves as such in all cases, even if we changed the
  code to use a batch call or a multicall. It was also generating
  noise when listening to notifications through a WebSocket.
- third, and this might be a direct consequence of the second point,
  the download queue order, or priority queue, was changed when using
  pause_all and/or resume_all.

Using the native pauseAll and unpauseAll methods of the RPC API, the
queue order now stays the same, and API.pause_all and API.resume_all
behave as expected, returning proper results (i.e. always True or
exception raised).
  • Loading branch information
pawamoy committed Oct 10, 2019
1 parent 2d018a3 commit 0bf2209
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 10 deletions.
14 changes: 6 additions & 8 deletions src/aria2p/api.py
Expand Up @@ -409,13 +409,11 @@ def pause_all(self, force=False):
Returns:
bool: Success or failure of the operation to pause all downloads.
"""
# if force:
# pause_func = self.client.force_pause_all
# else:
# pause_func = self.client.pause_all
# return pause_func() == "OK"

return all(self.pause([d for d in self.get_downloads() if d.status in ("active", "waiting")], force=force))
if force:
pause_func = self.client.force_pause_all
else:
pause_func = self.client.pause_all
return pause_func() == "OK"

def resume(self, downloads):
"""
Expand Down Expand Up @@ -449,7 +447,7 @@ def resume_all(self):
Returns:
bool: Success or failure of the operation to resume all downloads.
"""
return all(self.resume(self.get_downloads()))
return self.client.unpause_all() == "OK"

def autopurge(self):
"""
Expand Down
4 changes: 2 additions & 2 deletions tests/test_cli.py
Expand Up @@ -169,9 +169,9 @@ def test_resume_all_subcommand():
assert cli.subcommand_resume(server.api, do_all=True) == 0


def test_resume_all_subcommand_fails():
def test_resume_all_subcommand_doesnt_fail_with_already_active_downloads():
with Aria2Server(port=7519, session=SESSIONS_DIR / "dl-2-aria2.txt") as server:
assert cli.subcommand_resume(server.api, do_all=True) == 1
assert cli.subcommand_resume(server.api, do_all=True) == 0


def test_remove_subcommand():
Expand Down

0 comments on commit 0bf2209

Please sign in to comment.