Skip to content

feat: improve task polling with exponential backoff #408

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

Merged
merged 1 commit into from
May 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/posit/connect/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

from __future__ import annotations

import time

from typing_extensions import overload

from . import resources
Expand Down Expand Up @@ -95,17 +97,40 @@ def update(self, *args, **kwargs) -> None:
result = response.json()
super().update(**result)

def wait_for(self) -> None:
def wait_for(self, *, initial_wait: int = 1, max_wait: int = 10, backoff: float = 1.5) -> None:
"""Wait for the task to finish.

Parameters
----------
initial_wait : int, default 1
Initial wait time in seconds. First API request will use this as the wait parameter.
max_wait : int, default 10
Maximum wait time in seconds between polling requests.
backoff : float, default 1.5
Backoff multiplier for increasing wait times.

Examples
--------
>>> task.wait_for()
None

Notes
-----
This method implements an exponential backoff strategy to reduce the number of API calls
while waiting for long-running tasks. The first request uses the initial_wait value,
and subsequent requests increase the wait time by the backoff factor, up to max_wait. To disable exponential backoff, set backoff to 1.0.
"""
wait_time = initial_wait

while not self.is_finished:
self.update()

# Wait client-side
time.sleep(wait_time)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was trying to suggest using the wait=N query argument to implement long-polling and not do any sleeping on the client.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that the sdk's http client is not async, I am curious what relying on long polling would gain here. Could be mistaken but whether you have a long standing connection or sleep, both are blocking until the response is returned right? If that is accurate then having the blocking driven by the client may still give this a slight edge.

Copy link
Collaborator

Choose a reason for hiding this comment

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

when you call /tasks/DECAFBAD?wait=10, the API call will wait up to ten seconds or until the task is complete before returning. read that call as "accumulate data for up to 10 seconds before returning the result".

the call returns as soon as the task is complete.

when the client is sleeping, it has to wait the entire duration before it can make an API call to fetch additional data. if the task completes after 1s, you still need to wait the remaining 9s.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the rsconnect R package uses wait=1 to provide fairly fast feedback while not issuing an excessive number of HTTP requests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the call returns as soon as the task is complete.
that makes sense then! I didnt know that it would return early.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I completely spaced on the API functionality for sleeps. I'll modify the implementation to use it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Much better :) #409


# Calculate next wait time with backoff
wait_time = min(wait_time * backoff, max_wait)


class Tasks(resources.Resources):
@overload
Expand Down
78 changes: 78 additions & 0 deletions tests/posit/connect/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,84 @@ def test(self):
assert mock_tasks_get[0].call_count == 1
assert mock_tasks_get[1].call_count == 1

@responses.activate
@mock.patch("time.sleep", autospec=True)
def test_exponential_backoff(self, mock_sleep):
uid = "jXhOhdm5OOSkGhJw"

# behavior
mock_tasks_get = [
responses.get(
f"https://connect.example/__api__/v1/tasks/{uid}",
json={**load_mock_dict(f"v1/tasks/{uid}.json"), "finished": False},
),
responses.get(
f"https://connect.example/__api__/v1/tasks/{uid}",
json={**load_mock_dict(f"v1/tasks/{uid}.json"), "finished": False},
),
responses.get(
f"https://connect.example/__api__/v1/tasks/{uid}",
json={**load_mock_dict(f"v1/tasks/{uid}.json"), "finished": False},
),
responses.get(
f"https://connect.example/__api__/v1/tasks/{uid}",
json={**load_mock_dict(f"v1/tasks/{uid}.json"), "finished": True},
),
]

# setup
c = connect.Client("https://connect.example", "12345")
task = c.tasks.get(uid)
assert not task.is_finished

# invoke
task.wait_for(initial_wait=1, max_wait=5, backoff=2.0)

# assert
assert task.is_finished
assert mock_tasks_get[0].call_count == 1
assert mock_tasks_get[1].call_count == 1

# Verify sleep calls
mock_sleep.assert_has_calls([mock.call(1), mock.call(2), mock.call(4)], any_order=False)

@responses.activate
@mock.patch("time.sleep", autospec=True)
def test_no_backoff(self, mock_sleep):
uid = "jXhOhdm5OOSkGhJw"

# behavior
mock_tasks_get = [
responses.get(
f"https://connect.example/__api__/v1/tasks/{uid}",
json={**load_mock_dict(f"v1/tasks/{uid}.json"), "finished": False},
),
responses.get(
f"https://connect.example/__api__/v1/tasks/{uid}",
json={**load_mock_dict(f"v1/tasks/{uid}.json"), "finished": False},
),
responses.get(
f"https://connect.example/__api__/v1/tasks/{uid}",
json={**load_mock_dict(f"v1/tasks/{uid}.json"), "finished": True},
),
]

# setup
c = connect.Client("https://connect.example", "12345")
task = c.tasks.get(uid)
assert not task.is_finished

# invoke
task.wait_for(initial_wait=2, max_wait=5, backoff=1.0)

# assert
assert task.is_finished
assert mock_tasks_get[0].call_count == 1
assert mock_tasks_get[1].call_count == 1

# Verify sleep calls
mock_sleep.assert_has_calls([mock.call(2), mock.call(2)], any_order=False)


class TestTasksGet:
@responses.activate
Expand Down