Address PR comments

Author: vkarak

reframe/core/schedulers/__init__.py

@@ -100,6 +100,57 @@ def filternodes(self, job, nodes):
         :meta private:
         '''
 
+    def filternodes_by_state(self, nodelist, state):
+        '''Filter nodes by their state
+
+        :arg nodelist: List of :class:`Node` instances to filter.
+        :arg state: The state of the nodes.
+            If ``all``, the initial list is returned untouched.
+            If ``avail``, only the available nodes will be returned.
+            All other values are interpreted as a state string.
+            The pipe character ``|`` can be used as to specify multiple
+            alternative node states.
+            State match is exclusive unless the ``*`` is added at the end of the
+            state string.
+            When defining multiple states using ``|``, ``*`` has to be added at
+            the end of each alternative state for which a non-exclusive match is
+            required.
+
+        :returns: the filtered node list
+
+        .. versionchanged:: 4.9
+
+           Support the ``|`` character to filter according to alternative states.
+
+        .. versionchanged:: 4.10
+
+           Moved inside the :class:`JobShceduler` class.
+        '''
+        if '|' in state:
+            allowed_states = state.split('|')
+            final_nodelist = set()
+            for s in allowed_states:
+                final_nodelist.update(
+                    self.filternodes_by_state(nodelist, s)
+                )
+
+            nodelist = final_nodelist
+        elif state == 'avail':
+            nodelist = {n for n in nodelist if self.is_node_avail(n)}
+        elif state != 'all':
+            if state.endswith('*'):
+                # non-exclusive state match
+                state = state[:-1]
+                nodelist = {
+                    n for n in nodelist if n.in_state(state)
+                }
+            else:
+                nodelist = {
+                    n for n in nodelist if n.in_statex(state)
+                }
+
+        return nodelist
+
     @abc.abstractmethod
     def submit(self, job):
         '''Submit a job.
@@ -153,53 +204,6 @@ def log(self, message, level=DEBUG2):
         getlogger().log(level, f'[S] {self.registered_name}: {message}')
 
 
-def filter_nodes_by_state(nodelist, state, scheduler):
-    '''Filter nodes by their state
-
-    :arg nodelist: List of :class:`Node` instances to filter.
-    :arg state: The state of the nodes.
-        If ``all``, the initial list is returned untouched.
-        If ``avail``, only the available nodes will be returned.
-        All other values are interpreted as a state string.
-        The pipe character ``|`` can be used as to specify multiple
-        alternative node states.
-        State match is exclusive unless the ``*`` is added at the end of the
-        state string.
-        When defining multiple states using ``|``, ``*`` has to be added at
-        the end of each alternative state for which a non-exclusive match is
-        required.
-
-    :returns: the filtered node list
-
-    .. versionchanged:: 4.9
-       Support the ``|`` character to filter according to alternative states.
-    '''
-    if '|' in state:
-        allowed_states = state.split('|')
-        final_nodelist = set()
-        for s in allowed_states:
-            final_nodelist.update(
-                filter_nodes_by_state(nodelist, s, scheduler)
-            )
-
-        nodelist = final_nodelist
-    elif state == 'avail':
-        nodelist = {n for n in nodelist if scheduler.is_node_avail(n)}
-    elif state != 'all':
-        if state.endswith('*'):
-            # non-exclusive state match
-            state = state[:-1]
-            nodelist = {
-                n for n in nodelist if n.in_state(state)
-            }
-        else:
-            nodelist = {
-                n for n in nodelist if n.in_statex(state)
-            }
-
-    return nodelist
-
-
 class Job(jsonext.JSONSerializable, metaclass=JobMeta):
     '''A job descriptor.
 
@@ -627,10 +631,9 @@ def guess_num_tasks(self):
         )
 
         # Try to guess the number of tasks now
-        available_nodes = filter_nodes_by_state(
+        available_nodes = self.scheduler.filternodes_by_state(
             available_nodes,
-            self.sched_flex_alloc_nodes.lower(),
-            self.scheduler
+            self.sched_flex_alloc_nodes.lower()
         )
         getlogger().debug(
             f'[F] Total available in state='

reframe/core/schedulers/slurm.py

@@ -147,7 +147,7 @@ def __init__(self):
         self._sched_access_in_submit = self.get_option(
             'sched_access_in_submit'
         )
-        self.node_available_states = {
+        self._available_states = {
             'ALLOCATED',
             'COMPLETING',
             'IDLE',
@@ -436,6 +436,8 @@ def filternodes(self, job, nodes):
 
     def _get_reservation_nodes(self, resv):
         completed = _run_strict(f'scontrol -a show -o reservations {resv}')
+        self.log(f'reservation info:\n{completed.stdout}')
+
         node_match = re.search(r'Nodes=(\S+)', completed.stdout)
         if node_match:
             reservation_nodes = node_match[1]
@@ -446,10 +448,7 @@ def _get_reservation_nodes(self, resv):
         flags_match = re.search(r'Flags=(\S+)', completed.stdout)
         if flags_match:
             if 'MAINT' in flags_match.group(1).split(','):
-                self.node_available_states.add('MAINTENANCE')
-        else:
-            self.log('could not extract the reservation flags for '
-                     f'reservation {resv!r}')
+                self._available_states.add('MAINTENANCE')
 
         completed = _run_strict(
             f'scontrol -a show -o nodes {reservation_nodes}'
@@ -648,7 +647,7 @@ def finished(self, job):
         return slurm_state_completed(job.state)
 
     def is_node_avail(self, node):
-        return node.states <= self.node_available_states
+        return node.states <= self._available_states
 
     def is_node_down(self, node):
         return not self.is_node_avail(node)

reframe/frontend/testgenerators.py

@@ -14,7 +14,7 @@
 from reframe.core.fields import make_convertible
 from reframe.core.logging import getlogger, time_function
 from reframe.core.meta import make_test
-from reframe.core.schedulers import Job, filter_nodes_by_state
+from reframe.core.schedulers import Job
 from reframe.frontend.executors import generate_testcases
 
 
@@ -38,10 +38,8 @@ def getallnodes(state, jobs_cli_options=None):
             f'Total available nodes for {part.name}: {len(available_nodes)}'
         )
 
-        available_nodes = filter_nodes_by_state(
-            available_nodes,
-            state,
-            part.scheduler
+        available_nodes = part.scheduler.filternodes_by_state(
+            available_nodes, state
         )
         nodes[part.fullname] = [n.name for n in available_nodes]