Skip to content
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
6 changes: 5 additions & 1 deletion hyperbrowser/client/managers/async_manager/sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,11 @@ async def update_network(
payload = await self._request(
"PUT",
f"/sandbox/{sandbox_id}/network",
data=policy.model_dump(exclude_none=True, by_alias=True),
data=policy.model_dump(
exclude_none=True,
exclude_unset=True,
by_alias=True,
),
)
return SandboxNetworkUpdateResult(**payload)

Expand Down
6 changes: 5 additions & 1 deletion hyperbrowser/client/managers/sync_manager/sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,11 @@ def update_network(
payload = self._request(
"PUT",
f"/sandbox/{sandbox_id}/network",
data=policy.model_dump(exclude_none=True, by_alias=True),
data=policy.model_dump(
exclude_none=True,
exclude_unset=True,
by_alias=True,
),
Comment on lines +627 to +631

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Behavioral change to update_network wire format may affect server-side interpretation

This PR changes the wire format of update_network requests: previously, calling update_network(SandboxNetworkPolicy(allow_internet_access=True)) would send {"allowInternetAccess": true, "allowOut": [], "denyOut": []} (because allow_out and deny_out default to [] via default_factory=list at hyperbrowser/models/sandbox.py:118-119). Now it sends only {"allowInternetAccess": true}. This is a semantic change from "replace the entire network policy" to "partially update only the specified fields." The server must support partial/patch-like semantics on this PUT endpoint for this to work correctly. The new tests confirm this is intentional, but it's worth verifying the server-side contract.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

)
return SandboxNetworkUpdateResult(**payload)

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "hyperbrowser"
version = "0.93.0"
version = "0.93.1"
description = "Python SDK for hyperbrowser"
authors = ["Nikhil Shahi <nshahi1998@gmail.com>"]
license = "MIT"
Expand Down
22 changes: 22 additions & 0 deletions tests/test_create_sandbox_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,28 @@ def test_sandbox_network_policy_serializes_update_payload():
}


def test_sandbox_network_policy_defaults_unset_lists_for_response_models():
policy = SandboxNetworkPolicy(allow_internet_access=True)

assert policy.model_dump(by_alias=True, exclude_none=True) == {
"allowInternetAccess": True,
"allowOut": [],
"denyOut": [],
}


def test_sandbox_network_policy_can_omit_unset_lists_for_patch_payload():
policy = SandboxNetworkPolicy(allow_internet_access=True)

assert policy.model_dump(
by_alias=True,
exclude_none=True,
exclude_unset=True,
) == {
"allowInternetAccess": True,
}


def test_sandbox_image_build_params_serialize_expected_wire_keys():
create_params = CreateSandboxImageBuildParams(
image_name="custom_node",
Expand Down
55 changes: 55 additions & 0 deletions tests/test_sandbox_wire_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,12 @@ def test_sandbox_request_models_serialize_expected_wire_keys():
"allowOut": [],
"denyOut": [],
}
assert SandboxNetworkPolicy(allow_internet_access=True).model_dump(
by_alias=True, exclude_none=True
) == {"allowInternetAccess": True, "allowOut": [], "denyOut": []}
assert SandboxNetworkPolicy(allow_internet_access=True).model_dump(
by_alias=True, exclude_none=True, exclude_unset=True
) == {"allowInternetAccess": True}

assert SandboxImageListParams(
page=2,
Expand Down Expand Up @@ -1074,6 +1080,30 @@ def test_sync_sandbox_control_manager_uses_expected_wire_keys():
assert unexpose_call["url"].endswith("/sandbox/sbx_123/unexpose")


def test_sync_sandbox_update_network_omits_only_unset_lists():
client = FakeSyncClient()
manager = SandboxManager(client)
sandbox = manager.attach(SandboxDetail(**SANDBOX_DETAIL_PAYLOAD))

sandbox.update_network(SandboxNetworkPolicy(allow_internet_access=True))
sandbox.update_network(
SandboxNetworkPolicy(
allow_internet_access=True,
allow_out=[],
deny_out=[],
)
)

preserve_call = client.transport.client.calls[0]
clear_call = client.transport.client.calls[1]
assert preserve_call["json"] == {"allowInternetAccess": True}
assert clear_call["json"] == {
"allowInternetAccess": True,
"allowOut": [],
"denyOut": [],
}


def test_snapshot_summary_allows_missing_compatibility_tag():
snapshot = SandboxSnapshotSummary(**SNAPSHOT_PAYLOAD_WITHOUT_COMPATIBILITY_TAG)

Expand Down Expand Up @@ -1528,6 +1558,31 @@ async def test_async_sandbox_control_manager_uses_expected_wire_keys():
assert unexpose_call["json"] == {"port": 3000}


@pytest.mark.anyio
async def test_async_sandbox_update_network_omits_only_unset_lists():
client = FakeAsyncClient()
manager = AsyncSandboxManager(client)
sandbox = manager.attach(SandboxDetail(**SANDBOX_DETAIL_PAYLOAD))

await sandbox.update_network(SandboxNetworkPolicy(allow_internet_access=True))
await sandbox.update_network(
SandboxNetworkPolicy(
allow_internet_access=True,
allow_out=[],
deny_out=[],
)
)

preserve_call = client.transport.client.calls[0]
clear_call = client.transport.client.calls[1]
assert preserve_call["json"] == {"allowInternetAccess": True}
assert clear_call["json"] == {
"allowInternetAccess": True,
"allowOut": [],
"denyOut": [],
}


@pytest.mark.anyio
async def test_async_sandbox_runtime_apis_use_expected_wire_keys():
transport = AsyncRecordingTransport()
Expand Down