From a8622e6ab87ce6944f38d2b897766ef14ce3cad0 Mon Sep 17 00:00:00 2001 From: Tamir Date: Fri, 27 Mar 2026 13:09:48 +0200 Subject: [PATCH 1/3] refactor Volume class to use dataclass and dataclass_json Replaces the manual __init__/property/create_from_dict pattern with @dataclass + @dataclass_json for consistency with the Instance class. Undefined.EXCLUDE silently drops unknown API fields; infer_missing defaults missing optional fields to None. Retains create_from_dict as a deprecated shim for backwards compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 4 + tests/unit_tests/volumes/test_volumes.py | 24 +- verda/volumes/_volumes.py | 395 ++++------------------- 3 files changed, 77 insertions(+), 346 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56ececa..f099794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added missing fields to the `Volume` class: `pseudo_path`, `mount_command`, `create_directory_command`, `filesystem_to_fstab_command`, `instances`, `contract`, `base_hourly_cost`, `monthly_price`, `currency`, `long_term` +### Changed + +- Refactored `Volume` class to use `@dataclass` and `@dataclass_json` for consistency with `Instance` class. `Volume.create_from_dict()` is replaced by `Volume.from_dict()`; unknown API fields are now silently ignored via `Undefined.EXCLUDE` + ## [1.23.1] - 2026-03-25 ### Fixed diff --git a/tests/unit_tests/volumes/test_volumes.py b/tests/unit_tests/volumes/test_volumes.py index 1c45950..03718d8 100644 --- a/tests/unit_tests/volumes/test_volumes.py +++ b/tests/unit_tests/volumes/test_volumes.py @@ -49,7 +49,7 @@ 'is_os_volume': True, 'created_at': NVME_VOL_CREATED_AT, 'target': TARGET_VDA, - 'ssh_key_ids': SSH_KEY_ID, + 'ssh_key_ids': [SSH_KEY_ID], 'pseudo_path': 'volume-nxC2tf9F', 'mount_command': 'mount -t nfs -o nconnect=16 nfs.fin-01.datacrunch.io:volume-nxC2tf9F /mnt/volume', 'create_directory_command': 'mkdir -p /mnt/volume', @@ -108,13 +108,13 @@ def endpoint(self, http_client): def test_initialize_a_volume(self): volume = Volume( - RANDOM_VOL_ID, - VolumeStatus.DETACHED, - HDD_VOL_NAME, - HDD_VOL_SIZE, - HDD, - False, - HDD_VOL_CREATED_AT, + id=RANDOM_VOL_ID, + status=VolumeStatus.DETACHED, + name=HDD_VOL_NAME, + size=HDD_VOL_SIZE, + type=HDD, + is_os_volume=False, + created_at=HDD_VOL_CREATED_AT, ) assert volume.id == RANDOM_VOL_ID @@ -129,8 +129,8 @@ def test_initialize_a_volume(self): assert volume.target is None assert volume.ssh_key_ids == [] - def test_create_from_dict_without_new_fields(self): - """Test that create_from_dict handles API responses missing the new fields.""" + def test_from_dict_without_optional_fields(self): + """Test that from_dict handles API responses missing optional fields.""" minimal_dict = { 'id': RANDOM_VOL_ID, 'status': VolumeStatus.DETACHED, @@ -144,7 +144,7 @@ def test_create_from_dict_without_new_fields(self): 'instance_id': None, 'ssh_key_ids': [], } - volume = Volume.create_from_dict(minimal_dict) + volume = Volume.from_dict(minimal_dict, infer_missing=True) assert volume.id == RANDOM_VOL_ID assert volume.pseudo_path is None assert volume.mount_command is None @@ -181,7 +181,7 @@ def test_get_volumes(self, volumes_service, endpoint): assert volume_nvme.is_os_volume assert volume_nvme.created_at == NVME_VOL_CREATED_AT assert volume_nvme.target == TARGET_VDA - assert volume_nvme.ssh_key_ids == SSH_KEY_ID + assert volume_nvme.ssh_key_ids == [SSH_KEY_ID] assert volume_nvme.pseudo_path == NVME_VOLUME['pseudo_path'] assert volume_nvme.mount_command == NVME_VOLUME['mount_command'] assert volume_nvme.create_directory_command == NVME_VOLUME['create_directory_command'] diff --git a/verda/volumes/_volumes.py b/verda/volumes/_volumes.py index e896b7f..1fcccc3 100644 --- a/verda/volumes/_volumes.py +++ b/verda/volumes/_volumes.py @@ -1,346 +1,73 @@ +from dataclasses import dataclass, field + +from dataclasses_json import Undefined, dataclass_json + from verda.constants import Locations, VolumeActions -from verda.helpers import stringify_class_object_properties VOLUMES_ENDPOINT = '/volumes' +@dataclass_json(undefined=Undefined.EXCLUDE) +@dataclass class Volume: - """A volume model class.""" - - def __init__( - self, - id: str, - status: str, - name: str, - size: int, - type: str, - is_os_volume: bool, - created_at: str, - target: str | None = None, - location: str = Locations.FIN_03, - instance_id: str | None = None, - ssh_key_ids: list[str] = [], - deleted_at: str | None = None, - pseudo_path: str | None = None, - mount_command: str | None = None, - create_directory_command: str | None = None, - filesystem_to_fstab_command: str | None = None, - instances: list[dict] | None = None, - contract: str | None = None, - base_hourly_cost: float | None = None, - monthly_price: float | None = None, - currency: str | None = None, - long_term: dict | None = None, - ) -> None: - """Initialize the volume object. - - :param id: volume id - :type id: str - :param status: volume status - :type status: str - :param name: volume name - :type name: str - :param size: volume size in GB - :type size: int - :param type: volume type - :type type: str - :param is_os_volume: indication whether this is an operating systen volume - :type is_os_volume: bool - :param created_at: the time the volume was created (UTC) - :type created_at: str - :param target: target device e.g. vda - :type target: str, optional - :param location: datacenter location, defaults to "FIN-03" - :type location: str, optional - :param instance_id: the instance id the volume is attached to, None if detached - :type instance_id: str - :param ssh_key_ids: list of ssh keys ids - :type ssh_key_ids: list[str] - :param deleted_at: the time the volume was deleted (UTC), defaults to None - :type deleted_at: str, optional - :param pseudo_path: volume pseudo path for NFS export, defaults to None - :type pseudo_path: str, optional - :param mount_command: ready-to-use NFS mount command, defaults to None - :type mount_command: str, optional - :param create_directory_command: mkdir command for mount point, defaults to None - :type create_directory_command: str, optional - :param filesystem_to_fstab_command: fstab entry command for persistent mounts, defaults to None - :type filesystem_to_fstab_command: str, optional - :param instances: list of attached instance details, defaults to None - :type instances: list[dict], optional - :param contract: volume contract type e.g. "LONG_TERM", "PAY_AS_YOU_GO", defaults to None - :type contract: str, optional - :param base_hourly_cost: volume base hourly cost, defaults to None - :type base_hourly_cost: float, optional - :param monthly_price: volume monthly price, defaults to None - :type monthly_price: float, optional - :param currency: volume currency e.g. "usd", "eur", defaults to None - :type currency: str, optional - :param long_term: long term contract details, defaults to None - :type long_term: dict, optional - """ - self._id = id - self._status = status - self._name = name - self._size = size - self._type = type - self._is_os_volume = is_os_volume - self._created_at = created_at - self._target = target - self._location = location - self._instance_id = instance_id - self._ssh_key_ids = ssh_key_ids - self._deleted_at = deleted_at - self._pseudo_path = pseudo_path - self._mount_command = mount_command - self._create_directory_command = create_directory_command - self._filesystem_to_fstab_command = filesystem_to_fstab_command - self._instances = instances - self._contract = contract - self._base_hourly_cost = base_hourly_cost - self._monthly_price = monthly_price - self._currency = currency - self._long_term = long_term - - @property - def id(self) -> str: - """Get the volume id. - - :return: volume id - :rtype: str - """ - return self._id - - @property - def status(self) -> str: - """Get the volume status. - - :return: volume status - :rtype: str - """ - return self._status - - @property - def name(self) -> str: - """Get the volume name. - - :return: volume name - :rtype: str - """ - return self._name - - @property - def size(self) -> int: - """Get the volume size. - - :return: volume size - :rtype: int - """ - return self._size - - @property - def type(self) -> int: - """Get the volume type. - - :return: volume type - :rtype: string - """ - return self._type - - @property - def is_os_volume(self) -> bool: - """Return true iff the volume contains an operating system. - - :return: true iff the volume contains an OS - :rtype: bool - """ - return self._is_os_volume - - @property - def created_at(self) -> str: - """Get the time when the volume was created (UTC). - - :return: time - :rtype: str - """ - return self._created_at - - @property - def target(self) -> str | None: - """Get the target device. - - :return: target device - :rtype: str, optional - """ - return self._target + """Represents a storage volume with its configuration and state. + + Attributes: + id: Unique identifier for the volume. + status: Current status of the volume (e.g., 'attached', 'detached'). + name: Volume name. + size: Volume size in GB. + type: Volume type (e.g., 'NVMe', 'HDD', 'NVMe_Shared'). + is_os_volume: Whether this is an operating system volume. + created_at: Timestamp of volume creation (UTC). + target: Target device (e.g., 'vda'). + location: Datacenter location code. + instance_id: ID of the instance the volume is attached to, None if detached. + ssh_key_ids: List of SSH key IDs linked to the volume. + deleted_at: Timestamp of volume deletion (UTC). + pseudo_path: Volume pseudo path for NFS export. + mount_command: Ready-to-use NFS mount command. + create_directory_command: mkdir command for creating the mount point directory. + filesystem_to_fstab_command: fstab entry command for persistent mounts. + instances: List of attached instance details. + contract: Volume contract type (e.g., 'LONG_TERM', 'PAY_AS_YOU_GO'). + base_hourly_cost: Volume base hourly cost. + monthly_price: Volume monthly price. + currency: Volume currency (e.g., 'usd', 'eur'). + long_term: Long term contract details. + """ + + id: str + status: str + name: str + size: int + type: str + is_os_volume: bool + created_at: str + target: str | None = None + location: str = Locations.FIN_03 + instance_id: str | None = None + ssh_key_ids: list[str] = field(default_factory=list) + deleted_at: str | None = None + pseudo_path: str | None = None + mount_command: str | None = None + create_directory_command: str | None = None + filesystem_to_fstab_command: str | None = None + instances: list[dict] | None = None + contract: str | None = None + base_hourly_cost: float | None = None + monthly_price: float | None = None + currency: str | None = None + long_term: dict | None = None - @property - def location(self) -> str: - """Get the volume datacenter location. - - :return: datacenter location - :rtype: str - """ - return self._location - - @property - def instance_id(self) -> str | None: - """Get the instance id the volume is attached to, if attached. Otherwise None. - - :return: instance id if attached, None otherwise - :rtype: str, optional - """ - return self._instance_id - - @property - def ssh_key_ids(self) -> list[str]: - """Get the SSH key IDs of the instance. - - :return: SSH key IDs - :rtype: list[str] - """ - return self._ssh_key_ids - - @property - def deleted_at(self) -> str | None: - """Get the time when the volume was deleted (UTC). - - :return: time - :rtype: str - """ - return self._deleted_at - - @property - def pseudo_path(self) -> str | None: - """Get the volume pseudo path for NFS export. - - :return: volume pseudo path - :rtype: str, optional - """ - return self._pseudo_path - - @property - def mount_command(self) -> str | None: - """Get the ready-to-use NFS mount command. - - :return: mount command - :rtype: str, optional - """ - return self._mount_command - - @property - def create_directory_command(self) -> str | None: - """Get the mkdir command for creating the mount point directory. - - :return: create directory command - :rtype: str, optional - """ - return self._create_directory_command - - @property - def filesystem_to_fstab_command(self) -> str | None: - """Get the fstab entry command for persistent mounts. - - :return: fstab command - :rtype: str, optional - """ - return self._filesystem_to_fstab_command - - @property - def instances(self) -> list[dict] | None: - """Get the list of attached instance details. - - :return: list of instance details - :rtype: list[dict], optional - """ - return self._instances - - @property - def contract(self) -> str | None: - """Get the volume contract type. - - :return: contract type e.g. "LONG_TERM", "PAY_AS_YOU_GO" - :rtype: str, optional - """ - return self._contract - - @property - def base_hourly_cost(self) -> float | None: - """Get the volume base hourly cost. - - :return: base hourly cost - :rtype: float, optional - """ - return self._base_hourly_cost - - @property - def monthly_price(self) -> float | None: - """Get the volume monthly price. - - :return: monthly price - :rtype: float, optional - """ - return self._monthly_price - - @property - def currency(self) -> str | None: - """Get the volume currency. - - :return: currency e.g. "usd", "eur" - :rtype: str, optional - """ - return self._currency - - @property - def long_term(self) -> dict | None: - """Get the long term contract details. - - :return: long term contract details - :rtype: dict, optional - """ - return self._long_term @classmethod - def create_from_dict(cls: 'Volume', volume_dict: dict) -> 'Volume': + def create_from_dict(cls, volume_dict: dict) -> 'Volume': """Create a Volume object from a dictionary. - :param volume_dict: dictionary representing the volume - :type volume_dict: dict - :return: Volume - :rtype: Volume - """ - return cls( - id=volume_dict['id'], - status=volume_dict['status'], - name=volume_dict['name'], - size=volume_dict['size'], - type=volume_dict['type'], - is_os_volume=volume_dict['is_os_volume'], - created_at=volume_dict['created_at'], - target=volume_dict['target'], - location=volume_dict['location'], - instance_id=volume_dict['instance_id'], - ssh_key_ids=volume_dict['ssh_key_ids'], - deleted_at=volume_dict.get('deleted_at'), - pseudo_path=volume_dict.get('pseudo_path'), - mount_command=volume_dict.get('mount_command'), - create_directory_command=volume_dict.get('create_directory_command'), - filesystem_to_fstab_command=volume_dict.get('filesystem_to_fstab_command'), - instances=volume_dict.get('instances'), - contract=volume_dict.get('contract'), - base_hourly_cost=volume_dict.get('base_hourly_cost'), - monthly_price=volume_dict.get('monthly_price'), - currency=volume_dict.get('currency'), - long_term=volume_dict.get('long_term'), - ) - - def __str__(self) -> str: - """Returns a string of the json representation of the volume. - - :return: json representation of the volume - :rtype: str + .. deprecated:: Use :meth:`from_dict` instead. """ - return stringify_class_object_properties(self) + return cls.from_dict(volume_dict, infer_missing=True) class VolumesService: @@ -358,7 +85,7 @@ def get(self, status: str | None = None) -> list[Volume]: :rtype: list[Volume] """ volumes_dict = self._http_client.get(VOLUMES_ENDPOINT, params={'status': status}).json() - return list(map(Volume.create_from_dict, volumes_dict)) + return [Volume.from_dict(v, infer_missing=True) for v in volumes_dict] def get_by_id(self, id: str) -> Volume: """Get a specific volume by its. @@ -370,7 +97,7 @@ def get_by_id(self, id: str) -> Volume: """ volume_dict = self._http_client.get(VOLUMES_ENDPOINT + f'/{id}').json() - return Volume.create_from_dict(volume_dict) + return Volume.from_dict(volume_dict, infer_missing=True) def get_in_trash(self) -> list[Volume]: """Get all volumes that are in trash. @@ -380,7 +107,7 @@ def get_in_trash(self) -> list[Volume]: """ volumes_dicts = self._http_client.get(VOLUMES_ENDPOINT + '/trash').json() - return list(map(Volume.create_from_dict, volumes_dicts)) + return [Volume.from_dict(v, infer_missing=True) for v in volumes_dicts] def create( self, From 3c59906895e9f754f00f5100f49f4752905dfcf2 Mon Sep 17 00:00:00 2001 From: Tamir Date: Mon, 30 Mar 2026 13:34:24 +0300 Subject: [PATCH 2/3] drop infer_missing=True from Volume.from_dict calls All optional fields already have defaults on the dataclass, so infer_missing is unnecessary. Removing it gives stricter validation: a response missing a required field like id or name will raise immediately instead of silently setting None. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/unit_tests/volumes/test_volumes.py | 2 +- verda/volumes/_volumes.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit_tests/volumes/test_volumes.py b/tests/unit_tests/volumes/test_volumes.py index 03718d8..c8c3e32 100644 --- a/tests/unit_tests/volumes/test_volumes.py +++ b/tests/unit_tests/volumes/test_volumes.py @@ -144,7 +144,7 @@ def test_from_dict_without_optional_fields(self): 'instance_id': None, 'ssh_key_ids': [], } - volume = Volume.from_dict(minimal_dict, infer_missing=True) + volume = Volume.from_dict(minimal_dict) assert volume.id == RANDOM_VOL_ID assert volume.pseudo_path is None assert volume.mount_command is None diff --git a/verda/volumes/_volumes.py b/verda/volumes/_volumes.py index 1fcccc3..0041b4d 100644 --- a/verda/volumes/_volumes.py +++ b/verda/volumes/_volumes.py @@ -67,7 +67,7 @@ def create_from_dict(cls, volume_dict: dict) -> 'Volume': .. deprecated:: Use :meth:`from_dict` instead. """ - return cls.from_dict(volume_dict, infer_missing=True) + return cls.from_dict(volume_dict) class VolumesService: @@ -85,7 +85,7 @@ def get(self, status: str | None = None) -> list[Volume]: :rtype: list[Volume] """ volumes_dict = self._http_client.get(VOLUMES_ENDPOINT, params={'status': status}).json() - return [Volume.from_dict(v, infer_missing=True) for v in volumes_dict] + return [Volume.from_dict(v) for v in volumes_dict] def get_by_id(self, id: str) -> Volume: """Get a specific volume by its. @@ -97,7 +97,7 @@ def get_by_id(self, id: str) -> Volume: """ volume_dict = self._http_client.get(VOLUMES_ENDPOINT + f'/{id}').json() - return Volume.from_dict(volume_dict, infer_missing=True) + return Volume.from_dict(volume_dict) def get_in_trash(self) -> list[Volume]: """Get all volumes that are in trash. @@ -107,7 +107,7 @@ def get_in_trash(self) -> list[Volume]: """ volumes_dicts = self._http_client.get(VOLUMES_ENDPOINT + '/trash').json() - return [Volume.from_dict(v, infer_missing=True) for v in volumes_dicts] + return [Volume.from_dict(v) for v in volumes_dicts] def create( self, From 25d46aa853a43dff96dacb72fbf81809db45ee78 Mon Sep 17 00:00:00 2001 From: Tamir Date: Mon, 30 Mar 2026 13:37:38 +0300 Subject: [PATCH 3/3] fix ruff formatting in _volumes.py Co-Authored-By: Claude Opus 4.6 (1M context) --- verda/volumes/_volumes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/verda/volumes/_volumes.py b/verda/volumes/_volumes.py index 0041b4d..fb19a00 100644 --- a/verda/volumes/_volumes.py +++ b/verda/volumes/_volumes.py @@ -60,7 +60,6 @@ class Volume: currency: str | None = None long_term: dict | None = None - @classmethod def create_from_dict(cls, volume_dict: dict) -> 'Volume': """Create a Volume object from a dictionary.