fix(api): address instance type and helper review comments

Remove obsolete dynamic pricing fields from instance types.
Move shared payload cleanup into verda.helpers, reuse the shared Currency alias, update client service docstrings, and add helper coverage requested during review.

Co-authored-by: imagene-shahar <imagene-shahar@users.noreply.github.com>

Author: cursoragent

tests/unit_tests/instance_types/test_instance_types.py

@@ -20,7 +20,6 @@
 P2P = '300 GB/s'
 PRICE_PER_HOUR = 5.0
 SPOT_PRICE_PER_HOUR = 2.5
-MAX_DYNAMIC_PRICE = 7.5
 SERVERLESS_PRICE = 1.25
 SERVERLESS_SPOT_PRICE = 0.75
 INSTANCE_TYPE = '8V100.48M'
@@ -29,8 +28,6 @@
 DISPLAY_NAME = 'NVIDIA Tesla V100'
 SUPPORTED_OS = ['ubuntu-24.04-cuda-12.8-open-docker']
 
-PRICE_HISTORY_PAYLOAD = [{'date': '2024-01-01', 'price_per_hour': '2.00'}]
-
 
 @responses.activate
 def test_instance_types(http_client):
@@ -69,7 +66,6 @@ def test_instance_types(http_client):
                 'p2p': P2P,
                 'price_per_hour': '5.00',
                 'spot_price': '2.50',
-                'max_dynamic_price': '7.50',
                 'serverless_price': '1.25',
                 'serverless_spot_price': '0.75',
                 'instance_type': INSTANCE_TYPE,
@@ -106,7 +102,6 @@ def test_instance_types(http_client):
     assert instance_type.display_name == DISPLAY_NAME
     assert instance_type.supported_os == SUPPORTED_OS
     assert instance_type.deploy_warning == 'Use updated drivers'
-    assert instance_type.max_dynamic_price == MAX_DYNAMIC_PRICE
     assert instance_type.serverless_price == SERVERLESS_PRICE
     assert instance_type.serverless_spot_price == SERVERLESS_SPOT_PRICE
     assert isinstance(instance_type.cpu, dict)
@@ -123,22 +118,3 @@ def test_instance_types(http_client):
     assert instance_type.memory['size_in_gigabytes'] == MEMORY_SIZE
     assert instance_type.gpu_memory['size_in_gigabytes'] == GPU_MEMORY_SIZE
     assert instance_type.storage['size_in_gigabytes'] == STORAGE_SIZE
-
-
-@responses.activate
-def test_instance_type_price_history(http_client):
-    # arrange - add response mock
-    responses.add(
-        responses.GET,
-        http_client._base_url + '/instance-types/price-history',
-        json=PRICE_HISTORY_PAYLOAD,
-        status=200,
-    )
-
-    instance_types_service = InstanceTypesService(http_client)
-
-    # act
-    price_history = instance_types_service.get_price_history()
-
-    # assert
-    assert price_history == PRICE_HISTORY_PAYLOAD

tests/unit_tests/test_helpers.py

@@ -0,0 +1,27 @@
+from verda.helpers import strip_none_values
+
+
+def test_strip_none_values_removes_none_recursively():
+    data = {
+        'name': 'job',
+        'optional': None,
+        'nested': {
+            'keep': 'value',
+            'drop': None,
+        },
+        'items': [
+            {'keep': 1, 'drop': None},
+            None,
+            ['value', None],
+        ],
+    }
+
+    assert strip_none_values(data) == {
+        'name': 'job',
+        'nested': {'keep': 'value'},
+        'items': [
+            {'keep': 1},
+            None,
+            ['value', None],
+        ],
+    }

verda/_verda.py

@@ -88,13 +88,13 @@ def __init__(
         """Job deployments service. Deploy and manage serverless jobs"""
 
         self.container_types: ContainerTypesService = ContainerTypesService(self._http_client)
-        """Container types service. Get available serverless container SKUs"""
+        """Container types service. Get available serverless container info"""
 
         self.clusters: ClustersService = ClustersService(self._http_client)
         """Clusters service. Create and manage compute clusters"""
 
         self.cluster_types: ClusterTypesService = ClusterTypesService(self._http_client)
-        """Cluster types service. Get available cluster SKUs"""
+        """Cluster types service. Get available cluster info"""
 
         self.long_term: LongTermService = LongTermService(self._http_client)
         """Long-term service. Get available long-term pricing periods"""

verda/cluster_types/_cluster_types.py

@@ -1,11 +1,10 @@
 from dataclasses import dataclass
-from typing import Literal
 
 from dataclasses_json import dataclass_json
 
-CLUSTER_TYPES_ENDPOINT = '/cluster-types'
+from verda.constants import Currency
 
-Currency = Literal['usd', 'eur']
+CLUSTER_TYPES_ENDPOINT = '/cluster-types'
 
 
 @dataclass_json

verda/constants.py

@@ -1,3 +1,8 @@
+from typing import Literal
+
+Currency = Literal['usd', 'eur']
+
+
 class Actions:
     """Instance actions."""
 

verda/container_types/_container_types.py

@@ -1,11 +1,10 @@
 from dataclasses import dataclass
-from typing import Literal
 
 from dataclasses_json import dataclass_json
 
-CONTAINER_TYPES_ENDPOINT = '/container-types'
+from verda.constants import Currency
 
-Currency = Literal['usd', 'eur']
+CONTAINER_TYPES_ENDPOINT = '/container-types'
 
 
 @dataclass_json

verda/helpers.py

@@ -1,4 +1,5 @@
 import json
+from typing import Any
 
 
 def stringify_class_object_properties(class_object: type) -> str:
@@ -15,3 +16,12 @@ def stringify_class_object_properties(class_object: type) -> str:
         if property[:1] != '_' and type(getattr(class_object, property, '')).__name__ != 'method'
     }
     return json.dumps(class_properties, indent=2)
+
+
+def strip_none_values(data: Any) -> Any:
+    """Recursively remove ``None`` values from JSON-serializable data."""
+    if isinstance(data, dict):
+        return {key: strip_none_values(value) for key, value in data.items() if value is not None}
+    if isinstance(data, list):
+        return [strip_none_values(item) for item in data]
+    return data

verda/instance_types/_instance_types.py

@@ -1,11 +1,10 @@
 from dataclasses import dataclass
-from typing import Literal
 
 from dataclasses_json import dataclass_json
 
-INSTANCE_TYPES_ENDPOINT = '/instance-types'
+from verda.constants import Currency
 
-Currency = Literal['usd', 'eur']
+INSTANCE_TYPES_ENDPOINT = '/instance-types'
 
 
 @dataclass_json
@@ -24,6 +23,17 @@ class InstanceType:
         memory: Instance type memory details.
         gpu_memory: Instance type GPU memory details.
         storage: Instance type storage details.
+        best_for: Suggested use cases for the instance type.
+        model: GPU model.
+        name: Human-readable instance type name.
+        p2p: Peer-to-peer interconnect bandwidth details.
+        currency: Currency used for pricing.
+        manufacturer: Hardware manufacturer.
+        display_name: Display name shown to users.
+        supported_os: Supported operating system images.
+        deploy_warning: Optional deployment warning returned by the API.
+        serverless_price: Optional serverless price for the same hardware profile.
+        serverless_spot_price: Optional serverless spot price for the same hardware profile.
     """
 
     id: str
@@ -45,8 +55,6 @@ class InstanceType:
     display_name: str
     supported_os: list[str]
     deploy_warning: str | None = None
-    dynamic_price: float | None = None
-    max_dynamic_price: float | None = None
     serverless_price: float | None = None
     serverless_spot_price: float | None = None
 
@@ -88,16 +96,6 @@ def get(self, currency: Currency = 'usd') -> list[InstanceType]:
                 display_name=instance_type['display_name'],
                 supported_os=instance_type['supported_os'],
                 deploy_warning=instance_type.get('deploy_warning'),
-                dynamic_price=(
-                    float(instance_type['dynamic_price'])
-                    if instance_type.get('dynamic_price') is not None
-                    else None
-                ),
-                max_dynamic_price=(
-                    float(instance_type['max_dynamic_price'])
-                    if instance_type.get('max_dynamic_price') is not None
-                    else None
-                ),
                 serverless_price=(
                     float(instance_type['serverless_price'])
                     if instance_type.get('serverless_price') is not None
@@ -113,7 +111,3 @@ def get(self, currency: Currency = 'usd') -> list[InstanceType]:
         ]
 
         return instance_type_objects
-
-    def get_price_history(self):
-        """Get the deprecated dynamic price history endpoint as raw JSON."""
-        return self._http_client.get(f'{INSTANCE_TYPES_ENDPOINT}/price-history').json()

verda/job_deployments/_job_deployments.py

@@ -2,25 +2,16 @@
 
 from dataclasses import dataclass, field
 from enum import Enum
-from typing import Any
 
 from dataclasses_json import Undefined, dataclass_json
 
 from verda.containers import ComputeResource, Container, ContainerRegistrySettings
+from verda.helpers import strip_none_values
 from verda.http_client import HTTPClient
 
 JOB_DEPLOYMENTS_ENDPOINT = '/job-deployments'
 
 
-def _strip_none(data: Any) -> Any:
-    """Recursively remove None values from JSON-serializable data."""
-    if isinstance(data, dict):
-        return {key: _strip_none(value) for key, value in data.items() if value is not None}
-    if isinstance(data, list):
-        return [_strip_none(item) for item in data]
-    return data
-
-
 class JobDeploymentStatus(str, Enum):
     """Possible states of a job deployment."""
 
@@ -85,15 +76,15 @@ def create(self, deployment: JobDeployment) -> JobDeployment:
         """Create a new job deployment."""
         response = self._http_client.post(
             JOB_DEPLOYMENTS_ENDPOINT,
-            json=_strip_none(deployment.to_dict()),
+            json=strip_none_values(deployment.to_dict()),
         )
         return JobDeployment.from_dict(response.json(), infer_missing=True)
 
     def update(self, job_name: str, deployment: JobDeployment) -> JobDeployment:
         """Update an existing job deployment."""
         response = self._http_client.patch(
             f'{JOB_DEPLOYMENTS_ENDPOINT}/{job_name}',
-            json=_strip_none(deployment.to_dict()),
+            json=strip_none_values(deployment.to_dict()),
         )
         return JobDeployment.from_dict(response.json(), infer_missing=True)