-
Notifications
You must be signed in to change notification settings - Fork 2
Add execute and await completion function #643
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| from typing_extensions import Literal | ||
|
|
||
| import httpx | ||
| from uuid_utils import uuid7 | ||
|
|
||
| from .lsp import ( | ||
| LspResource, | ||
|
|
@@ -787,6 +788,59 @@ def execute( | |
| cast_to=DevboxAsyncExecutionDetailView, | ||
| ) | ||
|
|
||
| def execute_and_await_completion( | ||
| self, | ||
| devbox_id: str, | ||
| *, | ||
| command: str, | ||
| shell_name: Optional[str] | NotGiven = NOT_GIVEN, | ||
| polling_config: PollingConfig | None = None, | ||
| # The following are forwarded to the initial execute request | ||
| extra_headers: Headers | None = None, | ||
| extra_query: Query | None = None, | ||
| extra_body: Body | None = None, | ||
| timeout: float | httpx.Timeout | None | NotGiven = NOT_GIVEN, | ||
| idempotency_key: str | None = None, | ||
| ) -> DevboxAsyncExecutionDetailView: | ||
| """ | ||
| Execute a command and wait for it to complete with optimal latency for long running commands. | ||
|
|
||
| This method launches an execution with a generated command_id and first attempts to | ||
| return the result within the initial request's timeout. If the execution is not yet | ||
| complete, it switches to using wait_for_command to minimize latency while waiting. | ||
| """ | ||
| command_id = str(uuid7()) | ||
| execution = self.execute( | ||
| devbox_id, | ||
| command=command, | ||
| command_id=command_id, | ||
| shell_name=shell_name, | ||
| extra_headers=extra_headers, | ||
| extra_query=extra_query, | ||
| extra_body=extra_body, | ||
| timeout=timeout, | ||
| idempotency_key=idempotency_key, | ||
| ) | ||
| if execution.status == "completed": | ||
| return execution | ||
|
|
||
| def handle_timeout_error(error: Exception) -> DevboxAsyncExecutionDetailView: | ||
| if isinstance(error, APITimeoutError) or ( | ||
| isinstance(error, APIStatusError) and error.response.status_code == 408 | ||
| ): | ||
| return execution | ||
| raise error | ||
|
|
||
| def is_done(result: DevboxAsyncExecutionDetailView) -> bool: | ||
| return result.status == "completed" | ||
|
Comment on lines
+827
to
+835
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are re-used between the two functions, extract out?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about this, but Then I'd have to define another helper function to do call that extracted function with the execution, which isn't good. I'd rather keep |
||
|
|
||
| return poll_until( | ||
| lambda: self.wait_for_command(execution.execution_id, devbox_id=devbox_id, statuses=["completed"]), | ||
| is_done, | ||
| polling_config, | ||
| handle_timeout_error, | ||
| ) | ||
|
|
||
| def execute_async( | ||
| self, | ||
| id: str, | ||
|
|
@@ -2175,6 +2229,60 @@ async def execute( | |
| cast_to=DevboxAsyncExecutionDetailView, | ||
| ) | ||
|
|
||
| async def execute_and_await_completion( | ||
| self, | ||
| devbox_id: str, | ||
| *, | ||
| command: str, | ||
| shell_name: Optional[str] | NotGiven = NOT_GIVEN, | ||
| polling_config: PollingConfig | None = None, | ||
| # The following are forwarded to the initial execute request | ||
| extra_headers: Headers | None = None, | ||
| extra_query: Query | None = None, | ||
| extra_body: Body | None = None, | ||
| timeout: float | httpx.Timeout | None | NotGiven = NOT_GIVEN, | ||
| idempotency_key: str | None = None, | ||
| ) -> DevboxAsyncExecutionDetailView: | ||
| """ | ||
| Execute a command and wait for it to complete with optimal latency for long running commands. | ||
|
|
||
| This method launches an execution with a generated command_id and first attempts to | ||
| return the result within the initial request's timeout. If the execution is not yet | ||
| complete, it switches to using wait_for_command to minimize latency while waiting. | ||
| """ | ||
|
|
||
| command_id = str(uuid7()) | ||
| execution = await self.execute( | ||
| devbox_id, | ||
| command=command, | ||
| command_id=command_id, | ||
| shell_name=shell_name, | ||
| extra_headers=extra_headers, | ||
| extra_query=extra_query, | ||
| extra_body=extra_body, | ||
| timeout=timeout, | ||
| idempotency_key=idempotency_key, | ||
| ) | ||
| if execution.status == "completed": | ||
| return execution | ||
|
|
||
| def handle_timeout_error(error: Exception) -> DevboxAsyncExecutionDetailView: | ||
| if isinstance(error, APITimeoutError) or ( | ||
| isinstance(error, APIStatusError) and error.response.status_code == 408 | ||
| ): | ||
| return execution | ||
| raise error | ||
|
|
||
| def is_done(result: DevboxAsyncExecutionDetailView) -> bool: | ||
| return result.status == "completed" | ||
|
|
||
| return await async_poll_until( | ||
| lambda: self.wait_for_command(execution.execution_id, devbox_id=devbox_id, statuses=["completed"]), | ||
| is_done, | ||
| polling_config, | ||
| handle_timeout_error, | ||
| ) | ||
|
|
||
| async def execute_async( | ||
| self, | ||
| id: str, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,3 +61,37 @@ def test_tail_stdout_logs() -> None: | |
| if received: | ||
| break | ||
| assert isinstance(received, str) | ||
|
|
||
|
|
||
| @pytest.mark.timeout(30) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a test that doesn't use pollingConfig to make sure basecase is ok?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not necessary; the original test won't even use any of the polling config parameters |
||
| def test_execute_and_await_completion() -> None: | ||
| assert _devbox_id | ||
| completed = client.devboxes.execute_and_await_completion( | ||
| _devbox_id, | ||
| command="echo hello && sleep 1", | ||
| polling_config=PollingConfig(max_attempts=120, interval_seconds=2.0, timeout_seconds=10 * 60), | ||
| ) | ||
| assert completed.status == "completed" | ||
|
|
||
|
|
||
| @pytest.mark.timeout(90) | ||
| def test_execute_and_await_completion_long_running() -> None: | ||
| assert _devbox_id | ||
| completed = client.devboxes.execute_and_await_completion( | ||
| _devbox_id, | ||
| command="echo hello && sleep 70", | ||
| polling_config=PollingConfig(max_attempts=120, interval_seconds=2.0), | ||
| ) | ||
| assert completed.status == "completed" | ||
|
|
||
|
|
||
| # TODO: Uncomment this test when we fix timeouts for polling | ||
| # @pytest.mark.timeout(30) | ||
| # def test_execute_and_await_completion_timeout() -> None: | ||
| # assert _devbox_id | ||
| # with pytest.raises(PollingTimeout): | ||
| # client.devboxes.execute_and_await_completion( | ||
| # devbox_id=_devbox_id, | ||
| # command="echo hello && sleep 10", | ||
| # polling_config=PollingConfig(max_attempts=1, interval_seconds=2.0, timeout_seconds=3), | ||
| # ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth instantiating idempotency_key -> command_id = str(uuid7()) within fn? idk if too much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
command_idis for internal use;idempotency_keyshould be specified by the user