Backport: Few fixes and handle modification consistently (#300)

- handle modification of objects via `modify()` consistently
- use correct list of node states when modifying the node state
- remove `consumable_resource` attribute of Partition class again, logic is merged into the `select_type_parameters` attribute

Author: tazend

docs/reference/index.md

@@ -41,6 +41,9 @@ The `pyslurm` package is a wrapper around the Slurm C-API
 * Node API
     * [pyslurm.Node][]
     * [pyslurm.Nodes][]
+* Partition API
+    * [pyslurm.Partition][]
+    * [pyslurm.Partitions][]
 * New Exceptions
     * [pyslurm.RPCError][]
     * [pyslurm.PyslurmError][]

pyslurm/core/job/job.pyx

@@ -92,7 +92,6 @@ cdef class Jobs(dict):
 
         Raises:
             RPCError: When getting all the Jobs from the slurmctld failed.
-            MemoryError: If malloc fails to allocate memory.
         """
         cdef:
             dict passwd = {}
@@ -188,7 +187,7 @@ cdef class Jobs(dict):
         """Format the information as list of Job objects.
 
         Returns:
-            (list): List of Job objects
+            (list[pyslurm.Job]): List of Job objects
         """
         return list(self.values())
 
@@ -258,7 +257,6 @@ cdef class Job:
         Raises:
             RPCError: If requesting the Job information from the slurmctld was
                 not successful.
-            MemoryError: If malloc failed to allocate memory.
 
         Examples:
             >>> import pyslurm

pyslurm/core/job/step.pyx

@@ -279,15 +279,17 @@ cdef class JobStep:
         step_id = self.ptr.step_id.step_id
         verify_rpc(slurm_kill_job_step(self.job_id, step_id, 9))
 
-    def modify(self, changes):
+    def modify(self, JobStep changes):
         """Modify a job step.
 
         Implements the slurm_update_step RPC.
 
         Args:
             changes (pyslurm.JobStep):
-                Another JobStep object which contains all the changes that
-                should be applied to this instance.
+                Another JobStep object that contains all the changes to apply.
+                Check the `Other Parameters` of the JobStep class to see which
+                properties can be modified.
+
         Raises:
             RPCError: When updating the JobStep was not successful.
 

pyslurm/core/node.pxd

@@ -117,6 +117,9 @@ cdef class Node:
             Default CPU-Binding for the node
         state (str):
             State of the node
+        reason (str):
+            Reason for the Node, typically used along with updating the node
+            state
 
     Attributes:
         name (str):

pyslurm/core/partition.pxd

@@ -101,7 +101,6 @@ cdef class Partition:
             * total_cpus
             * total_nodes
             * select_type_parameters
-            * consumable_resource
 
     Attributes:
         name (str):
@@ -116,10 +115,8 @@ cdef class Partition:
             List of QoS which are allowed to execute Jobs
         alternate (str):
             Name of the alternate Partition in case a Partition is down.
-        consumable_resource (str):
-            The type of consumable resource used in the Partition.
         select_type_parameters (list[str]):
-            List of additional parameters passed to the select plugin used.
+            List of Select type parameters for the select plugin.
         cpu_binding (str):
             Default CPU-binding for Jobs that execute in a Partition.
         default_memory_per_cpu (int):

pyslurm/core/partition.pyx

@@ -143,18 +143,29 @@ cdef class Partitions(dict):
 
         return self
 
-    def set_state(self, state):
-        """Modify the State of all Partitions in this Collection.
+    def modify(self, changes):
+        """Modify all Partitions in a Collection.
 
         Args:
-            state (str):
-                Partition state to set
+            changes (pyslurm.Partition):
+                Another Partition object that contains all the changes to
+                apply. Check the `Other Parameters` of the Partition class to
+                see which properties can be modified.
 
         Raises:
-            RPCError: When updating the state failed
+            RPCError: When updating at least one Partition failed.
+
+        Examples:
+            >>> import pyslurm
+            >>>
+            >>> parts = pyslurm.Partitions.load()
+            >>> # Prepare the changes
+            >>> changes = pyslurm.Partition(state="DRAIN")
+            >>> # Apply the changes to all the partitions
+            >>> parts.modify(changes)
         """
         for part in self.values():
-            part.modify(state=state)
+            part.modify(changes)
 
     def as_list(self):
         """Format the information as list of Partition objects.
@@ -270,40 +281,30 @@ cdef class Partition:
         verify_rpc(slurm_create_partition(self.ptr))
         return self
 
-    def modify(self, **changes):
+    def modify(self, Partition changes):
         """Modify a Partition.
 
         Implements the slurm_update_partition RPC.
 
         Args:
-            **changes (Any):
-                Changes for the Partition. Almost every Attribute from a
-                Partition can be modified, except for:
-
-                * total_cpus
-                * total_nodes
-                * select_type_parameters
-                * consumable_resource
+            changes (pyslurm.Partition):
+                Another Partition object that contains all the changes to
+                apply. Check the `Other Parameters` of the Partition class to
+                see which properties can be modified.
 
         Raises:
-            ValueError: When no changes were specified or when a parsing error
-                occured.
             RPCError: When updating the Partition was not successful.
 
         Examples:
             >>> import pyslurm
             >>>
-            >>> # Modifying the maximum time limit
-            >>> mypart = pyslurm.Partition("normal")
-            >>> mypart.modify(max_time_limit="10-00:00:00")
-            >>>
-            >>> # Modifying the partition state
-            >>> mypart.modify(state="DRAIN")
+            >>> part = pyslurm.Partition.load("normal")
+            >>> # Prepare the changes
+            >>> changes = pyslurm.Partition(state="DRAIN")
+            >>> # Apply the changes to the "normal" Partition
+            >>> part.modify(changes)
         """
-        if not changes:
-            raise ValueError("No changes were specified")
-
-        cdef Partition part = Partition(**changes)
+        cdef Partition part = <Partition>changes
         part.name = self._error_or_name()
         verify_rpc(slurm_update_partition(part.ptr))
 
@@ -381,10 +382,6 @@ cdef class Partition:
     def alternate(self, val):
         cstr.fmalloc(&self.ptr.alternate, val)
 
-    @property
-    def consumable_resource(self):
-        return _select_type_int_to_cons_res(self.ptr.cr_type)
-
     @property
     def select_type_parameters(self):
         return _select_type_int_to_list(self.ptr.cr_type)
@@ -757,7 +754,7 @@ def _split_oversubscribe_str(val):
 def _select_type_int_to_list(stype):
     # The rest of the CR_* stuff are just some extra parameters to the select
     # plugin
-    out = []
+    out = _select_type_int_to_cons_res(stype)
 
     if stype & slurm.CR_OTHER_CONS_RES:
         out.append("OTHER_CONS_RES")
@@ -800,7 +797,7 @@ def _select_type_int_to_cons_res(stype):
     elif stype & slurm.CR_MEMORY:
         return "MEMORY"
     else:
-        return None
+        return []
 
 
 def _preempt_mode_str_to_int(mode):

pyslurm/db/job.pyx

@@ -282,14 +282,19 @@ cdef class Jobs(dict):
         return jobs
 
     @staticmethod
-    def modify(search_filter, db_connection=None, **changes):
+    def modify(search_filter, Job changes, db_connection=None):
         """Modify Slurm database Jobs.
 
         Implements the slurm_job_modify RPC.
 
         Args:
             search_filter (Union[pyslurm.db.JobSearchFilter, pyslurm.db.Jobs]):
                 A filter to decide which Jobs should be modified.
+            changes (pyslurm.db.Job):
+                Another [pyslurm.db.Job][] object that contains all the
+                changes to apply. Check the `Other Parameters` of the
+                [pyslurm.db.Job][] class to see which properties can be
+                modified.
             db_connection (pyslurm.db.Connection):
                 A Connection to the slurmdbd. By default, if no connection is
                 supplied, one will automatically be created internally. This
@@ -306,25 +311,21 @@ cdef class Jobs(dict):
                 be committed or rolled back by using the respective methods on
                 the connection object. This way, you have a chance to see
                 which Jobs were modified before you commit the changes.
-            **changes (Any):
-                Check the `Other Parameters` Section of [pyslurm.db.Job][] to
-                see what attributes can be modified.
 
         Returns:
             (list[int]): A list of Jobs that were modified
 
         Raises:
-            ValueError: When a parsing error occured or the Database
-                connection is not open
             RPCError: When a failure modifying the Jobs occurred.
 
         Examples:
             In its simplest form, you can do something like this:
 
             >>> import pyslurm
+            >>> 
             >>> search_filter = pyslurm.db.JobSearchFilter(ids=[9999])
-            >>> modified_jobs = pyslurm.db.Jobs.modify(
-            ...             search_filter, comment="A comment for the job")
+            >>> changes = pyslurm.db.Job(comment="A comment for the job")
+            >>> modified_jobs = pyslurm.db.Jobs.modify(search_filter, changes)
             >>> print(modified_jobs)
             >>> [9999]
 
@@ -334,11 +335,13 @@ cdef class Jobs(dict):
             connection object:
 
             >>> import pyslurm
-            >>> search_filter = pyslurm.db.JobSearchFilter(ids=[9999])
+            >>> 
             >>> db_conn = pyslurm.db.Connection.open()
+            >>> search_filter = pyslurm.db.JobSearchFilter(ids=[9999])
+            >>> changes = pyslurm.db.Job(comment="A comment for the job")
             >>> modified_jobs = pyslurm.db.Jobs.modify(
-            ...             search_filter, db_conn,
-            ...             comment="A comment for the job")
+            ...             search_filter, changes, db_conn)
+            >>> 
             >>> # Now you can first examine which Jobs have been modified
             >>> print(modified_jobs)
             >>> [9999]
@@ -348,7 +351,7 @@ cdef class Jobs(dict):
         """
 
         cdef:
-            Job job = Job(**changes)
+            Job job = <Job>changes
             JobSearchFilter jfilter
             Connection conn = <Connection>db_connection
             SlurmList response
@@ -499,23 +502,25 @@ cdef class Job:
 
         return out
 
-    def modify(self, db_connection=None, **changes):
+    def modify(self, changes, db_connection=None):
         """Modify a Slurm database Job.
 
         Args:
+            changes (pyslurm.db.Job):
+                Another [pyslurm.db.Job][] object that contains all the
+                changes to apply. Check the `Other Parameters` of the
+                [pyslurm.db.Job][] class to see which properties can be
+                modified.
             db_connection (pyslurm.db.Connection):
                 A slurmdbd connection. See
                 [pyslurm.db.Jobs.modify][pyslurm.db.job.Jobs.modify] for more
-                info
-            **changes (Any):
-                Check the `Other Parameters` Section of this class to see what
-                attributes can be modified.
+                info on this parameter.
 
         Raises:
             RPCError: When modifying the Job failed.
         """
         cdef JobSearchFilter jfilter = JobSearchFilter(ids=[self.id])
-        Jobs.modify(jfilter, db_connection, **changes)
+        Jobs.modify(jfilter, changes, db_connection)
 
     @property
     def account(self):

tests/integration/test_db_job.py

@@ -60,7 +60,8 @@ def test_modify(submit_job):
     util.wait(5)
 
     jfilter = pyslurm.db.JobSearchFilter(ids=[job.id])
-    pyslurm.db.Jobs.modify(jfilter, comment="test comment")
+    changes = pyslurm.db.Job(comment="test comment")
+    pyslurm.db.Jobs.modify(jfilter, changes)
 
     job = pyslurm.db.Job.load(job.id)
     assert job.comment == "test comment"
@@ -72,7 +73,8 @@ def test_modify_with_existing_conn(submit_job):
 
     conn = pyslurm.db.Connection.open()
     jfilter = pyslurm.db.JobSearchFilter(ids=[job.id])
-    pyslurm.db.Jobs.modify(jfilter, conn, comment="test comment")
+    changes = pyslurm.db.Job(comment="test comment")
+    pyslurm.db.Jobs.modify(jfilter, changes, conn)
 
     job = pyslurm.db.Job.load(job.id)
     assert job.comment != "test comment"

tests/integration/test_partition.py

@@ -51,19 +51,19 @@ def test_create_delete():
 def test_modify():
     part = Partitions.load().as_list()[0]
 
-    part.modify(default_time=120)
+    part.modify(Partition(default_time=120))
     assert Partition.load(part.name).default_time == 120
 
-    part.modify(default_time="1-00:00:00")
+    part.modify(Partition(default_time="1-00:00:00"))
     assert Partition.load(part.name).default_time == 24*60
 
-    part.modify(default_time="UNLIMITED")
+    part.modify(Partition(default_time="UNLIMITED"))
     assert Partition.load(part.name).default_time == "UNLIMITED"
 
-    part.modify(state="DRAIN")
+    part.modify(Partition(state="DRAIN"))
     assert Partition.load(part.name).state == "DRAIN"
 
-    part.modify(state="UP")
+    part.modify(Partition(state="UP"))
     assert Partition.load(part.name).state == "UP"