SIMPLE-6429 fix potentially mishandled data structures (#94)

* SIMPLE-6429 changed several functions to not modify shared parameters

* SIMPLE-6429 fixed more mishandled data structures and various other small issues

Author: daniel-valent

pyproject.toml

@@ -15,7 +15,7 @@ classifiers = [
     "Environment :: Console",
     "License :: OSI Approved :: Apache Software License",
     "Operating System :: OS Independent",
-    "Programming Language :: Python :: 3.8",
+    "Programming Language :: Python :: 3.12",
     "Topic :: System :: Networking",
 ]
 

virl2_client/models/annotation.py

@@ -365,10 +365,8 @@ def update(
         :param push_to_server: Whether to push the changes to the server.
             Defaults to True; should only be False when used by internal methods.
         """
-        if "type" not in annotation_data:
-            annotation_data["type"] = self._type
-        elif annotation_data["type"] != self._type:
-            raise ValueError("Can't update annotation type.")
+        if annotation_data.get("type") not in (None, self._type):
+            raise ValueError("Can't change annotation type.")
 
         # make sure all properties we want to update are valid
         for key, value in annotation_data.items():
@@ -397,9 +395,9 @@ def _set_annotation_property(self, key: str, val: Any) -> None:
     @check_stale
     def _set_annotation_properties(self, annotation_data: dict[str, Any]) -> None:
         """Update annotation properties server-side."""
-        if "type" not in annotation_data:
-            annotation_data["type"] = self._type
-        self._session.patch(url=self._url_for("annotation"), json=annotation_data)
+        self._session.patch(
+            url=self._url_for("annotation"), json=annotation_data | {"type": self._type}
+        )
 
 
 # ~~~~~< Annotation subclasses >~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

virl2_client/models/lab.py

@@ -1312,19 +1312,22 @@ def import_lab(self, topology: dict) -> None:
         self._handle_import_annotations(topology)
 
     @locked
-    def _import_lab(self, topology: dict) -> None:
+    def _import_lab(self, topology: dict, created: bool = False) -> None:
         """
         Replace lab properties with the given topology.
 
         :param topology: The topology to import.
+        :param created: The node create API endpoint returns data in the old format,
+            which would print an unnecessary old schema warning;
+            setting this flag to True skips that warning.
         :raises KeyError: If any property is missing in the topology.
         """
         lab_dict = topology.get("lab")
 
         if lab_dict is None:
             # If we just created the lab, we skip the warning, since the
             # lab post endpoint returns data in the old format
-            if not topology.pop("_created", False):
+            if not created:
                 warnings.warn(
                     "Labs created in older CML releases (schema version 0.0.5 or lower)"
                     " are deprecated. Use labs with schema version 0.1.0 or higher.",
@@ -1347,15 +1350,15 @@ def _handle_import_nodes(self, topology: dict) -> None:
 
         :param topology: The topology to import nodes from.
         """
-        for node in topology["nodes"]:
+        for node in topology["nodes"]:  # type: dict
             node_id = node["id"]
 
             if node_id in self._nodes:
                 raise ElementAlreadyExists("Node already exists")
 
-            interfaces = node.pop("interfaces", [])
             self._import_node(node_id, node)
 
+            interfaces = node.get("interfaces", [])
             if not interfaces:
                 continue
 
@@ -1477,10 +1480,12 @@ def _import_node(self, node_id: str, node_data: dict) -> Node:
         if "data" in node_data:
             node_data = node_data["data"]
 
-        node_data.pop("id", None)
-        state = node_data.pop("state", None)
-        node_data.pop("lab_id", None)
-        node_data.pop("boot_progress", None)
+        state = node_data.get("state", None)
+        node_data = {
+            key: node_data[key]
+            for key in node_data
+            if key not in ("id", "state", "lab_id", "boot_progress", "interfaces")
+        }
 
         for key in ("image_definition", "configuration"):
             if key not in node_data:
@@ -1503,11 +1508,15 @@ def _import_annotation(
         :param annotation_data: The data of the annotation.
         :returns: The imported Annotation object.
         """
-        annotation_data.pop("id", None)
-        ann_type = annotation_data.pop("type")
+        annotation_type = annotation_data["type"]
+        annotation_data = {
+            key: annotation_data[key]
+            for key in annotation_data
+            if key not in ("id", "type")
+        }
 
         annotation = self._create_annotation_local(
-            annotation_id, ann_type, **annotation_data
+            annotation_id, annotation_type, **annotation_data
         )
         return annotation
 
@@ -1647,12 +1656,12 @@ def _add_nodes(self, topology: dict, new_nodes, new_interfaces):
         :param new_nodes: Iterable of node IDs to be added.
         :param new_interfaces: Iterable of interface IDs to be added.
         """
-        for node in topology["nodes"]:
+        for node in topology["nodes"]:  # type: dict
             node_id = node["id"]
-            interfaces = node.pop("interfaces", [])
+            interfaces = node.get("interfaces", [])
             if node_id in new_nodes:
-                node = self._import_node(node_id, node)
-                _LOGGER.info(f"Added node {node}")
+                new_node = self._import_node(node_id, node)
+                _LOGGER.info(f"Added node {new_node}")
 
             if not interfaces:
                 continue

virl2_client/models/node.py

@@ -841,7 +841,9 @@ def sync_operational(self, response: dict[str, Any] = None):
         """
         Synchronize the operational state of the node.
 
-        :param response: The response from the server.
+        :param response: If the operational data was fetched from the server elsewhere,
+            it can be passed here to save an API call. Will be fetched automatically
+            otherwise.
         """
         if response is None:
             url = self._url_for("operational")

virl2_client/models/resource_pools.py

@@ -438,12 +438,14 @@ def _set_resource_pool_property(self, key: str, val: Any) -> None:
 
     def _set_resource_pool_properties(self, resource_pool_data: dict[str, Any]) -> None:
         """Helper method to set multiple properties on the server."""
-        for key in list(resource_pool_data):
-            # drop unmodifiable properties
-            if key in ("id", "template", "users", "user_pools"):
-                resource_pool_data.pop(key)
+        # drop unmodifiable properties
+        resource_pool_data_post = {
+            key: resource_pool_data[key]
+            for key in resource_pool_data
+            if key not in ("id", "template", "users", "user_pools")
+        }
         url = self._url_for("resource_pool")
-        self._session.patch(url, json=resource_pool_data)
+        self._session.patch(url, json=resource_pool_data_post)
 
 
 ResourcePools = Dict[str, ResourcePool]

virl2_client/virl2_client.py

@@ -732,9 +732,7 @@ def create_lab(
             wait_time=self.convergence_wait_time,
             resource_pool_manager=self.resource_pool_management,
         )
-        # This is just to skip a deprecation warning in _import_lab
-        result["_created"] = True
-        lab._import_lab(result)
+        lab._import_lab(result, created=True)
         self._labs[lab_id] = lab
         return lab