Skip to content
Open
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
58 changes: 57 additions & 1 deletion tests/unit_tests/instances/test_instances.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import copy
import json

import pytest
import responses

from verda.constants import Actions, ErrorCodes, Locations
from verda.constants import Actions, ErrorCodes, InstanceStatus, Locations
from verda.exceptions import APIException
from verda.instances import Instance, InstancesService, OSVolume

Expand Down Expand Up @@ -333,6 +334,61 @@ def test_create_instance_attached_os_volume_successful(self, instances_service,
assert responses.assert_call_count(endpoint, 1) is True
assert responses.assert_call_count(url, 1) is True

@pytest.mark.parametrize(
('wait_for_status', 'expected_status', 'expected_get_instance_call_count'),
[
(None, InstanceStatus.ORDERED, 1),
(InstanceStatus.ORDERED, InstanceStatus.ORDERED, 1),
(InstanceStatus.PROVISIONING, InstanceStatus.PROVISIONING, 2),
(lambda status: status != InstanceStatus.ORDERED, InstanceStatus.PROVISIONING, 2),
(InstanceStatus.RUNNING, InstanceStatus.RUNNING, 3),
],
)
def test_create_wait_for_status(
self,
instances_service,
endpoint,
wait_for_status,
expected_status,
expected_get_instance_call_count,
):
# arrange - add response mock
# create instance
responses.add(responses.POST, endpoint, body=INSTANCE_ID, status=200)
# First get instance by id - ordered
get_instance_url = endpoint + '/' + INSTANCE_ID
payload = copy.deepcopy(PAYLOAD[0])
payload['status'] = InstanceStatus.ORDERED
responses.add(responses.GET, get_instance_url, json=payload, status=200)
# Second get instance by id - provisioning
payload = copy.deepcopy(PAYLOAD[0])
payload['status'] = InstanceStatus.PROVISIONING
responses.add(responses.GET, get_instance_url, json=payload, status=200)
# Third get instance by id - running
payload = copy.deepcopy(PAYLOAD[0])
payload['status'] = InstanceStatus.RUNNING
responses.add(responses.GET, get_instance_url, json=payload, status=200)

# act
instance = instances_service.create(
instance_type=INSTANCE_TYPE,
image=OS_VOLUME_ID,
hostname=INSTANCE_HOSTNAME,
description=INSTANCE_DESCRIPTION,
wait_for_status=wait_for_status,
max_interval=0,
max_wait_time=1,
)

# assert
assert isinstance(instance, Instance)
assert instance.id == INSTANCE_ID
assert instance.status == expected_status
assert responses.assert_call_count(endpoint, 1) is True
assert (
responses.assert_call_count(get_instance_url, expected_get_instance_call_count) is True
)

def test_create_instance_failed(self, instances_service, endpoint):
# arrange - add response mock
responses.add(
Expand Down
11 changes: 10 additions & 1 deletion verda/instances/_instances.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import itertools
import time
from collections.abc import Callable
from dataclasses import dataclass
from typing import Literal

Expand Down Expand Up @@ -150,6 +151,7 @@ def create(
pricing: Pricing | None = None,
coupon: str | None = None,
*,
wait_for_status: str | Callable[[str], bool] | None = lambda s: s != InstanceStatus.ORDERED,
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The PR title/description says it adds InstancesService.create_nowait, but the diff only changes create() by introducing wait_for_status. Either add the described create_nowait method (e.g., a small wrapper calling create(..., wait_for_status=None)), or update the PR metadata to reflect the actual API change.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

Updated the description

Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

wait_for_status is typed as str | Callable[[str], bool], but the code/tests appear to use InstanceStatus values (and instance.status is compared to InstanceStatus). This should be typed consistently (e.g., InstanceStatus | Callable[[InstanceStatus], bool] | None) to avoid misleading API contracts and type-checking issues.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

InstanceStatus is a namespace for str constants. The actual type passed to wait_for_status is str and not InstanceStatus

max_wait_time: float = 180,
initial_interval: float = 0.5,
max_interval: float = 5,
Expand All @@ -172,6 +174,7 @@ def create(
contract: Optional contract type for the instance.
pricing: Optional pricing model for the instance.
coupon: Optional coupon code for discounts.
wait_for_status: Status to wait for the instance to reach, or callable that returns True when the desired status is reached. Default to any status other than ORDERED. If None, no wait is performed.
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

Grammar: change 'Default to' to 'Defaults to' for readability.

Suggested change
wait_for_status: Status to wait for the instance to reach, or callable that returns True when the desired status is reached. Default to any status other than ORDERED. If None, no wait is performed.
wait_for_status: Status to wait for the instance to reach, or callable that returns True when the desired status is reached. Defaults to any status other than ORDERED. If None, no wait is performed.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

The current docstring is based on and consistent with ClustersService.create

max_wait_time: Maximum total wait for the instance to start provisioning, in seconds (default: 180)
initial_interval: Initial interval, in seconds (default: 0.5)
max_interval: The longest single delay allowed between retries, in seconds (default: 5)
Expand Down Expand Up @@ -203,12 +206,18 @@ def create(
payload['pricing'] = pricing
id = self._http_client.post(INSTANCES_ENDPOINT, json=payload).text

if wait_for_status is None:
return self.get_by_id(id)

# Wait for instance to enter provisioning state with timeout
# TODO(shamrin) extract backoff logic, _clusters module has the same code
deadline = time.monotonic() + max_wait_time
for i in itertools.count():
instance = self.get_by_id(id)
if instance.status != InstanceStatus.ORDERED:
if callable(wait_for_status):
if wait_for_status(instance.status):
return instance
elif instance.status == wait_for_status:
return instance
Comment on lines +217 to 221
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

wait_for_status is typed as str | Callable[[str], bool], but the code/tests appear to use InstanceStatus values (and instance.status is compared to InstanceStatus). This should be typed consistently (e.g., InstanceStatus | Callable[[InstanceStatus], bool] | None) to avoid misleading API contracts and type-checking issues.

Suggested change
if callable(wait_for_status):
if wait_for_status(instance.status):
return instance
elif instance.status == wait_for_status:
return instance
status_value = getattr(instance.status, 'value', instance.status)
if callable(wait_for_status):
if wait_for_status(status_value):
return instance
else:
target_status = getattr(wait_for_status, 'value', wait_for_status)
if status_value == target_status:
return instance

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

InstanceStatus is a namespace for str constants. The actual type passed to wait_for_status is str and not InstanceStatus


now = time.monotonic()
Expand Down