Fix parsing user names in JobSearchFilter (#303)

Fixes #302

- also makes it clearer in the docs for some attributes which type is
  expected inside the lists.

Author: tazend

pyslurm/db/job.pxd

@@ -56,72 +56,72 @@ from pyslurm.db.tres cimport TrackableResources, TrackableResource
 
 
 cdef class JobSearchFilter:
-    """Search conditions for Slurm database Jobs.
+    """Query-Conditions for Jobs in the Slurm Database.
 
     Args:
         **kwargs (Any, optional=None):
             Any valid attribute of the object.
 
     Attributes:
-        ids (list):
+        ids (list[int]):
             A list of Job ids to search for.
         start_time (Union[str, int, datetime.datetime]):
             Search for Jobs which started after this time.
         end_time (Union[str, int, datetime.datetime]):
             Search for Jobs which ended before this time.
-        accounts (list):
+        accounts (list[str]):
             Search for Jobs with these account names.
-        association_ids (list):
+        association_ids (list[int]):
             Search for Jobs with these association ids.
-        clusters (list):
+        clusters (list[str]):
             Search for Jobs running in these clusters.
-        constraints (list):
+        constraints (list[str]):
             Search for Jobs with these constraints.
         cpus (int):
             Search for Jobs with exactly this many CPUs.
-            Note: If you also specify max_cpus, then this value will act as
+            Note: If you also specify `max_cpus`, then this value will act as
             the minimum.
         max_cpus (int):
             Search for Jobs with no more than this amount of CPUs.
-            Note: This value has no effect without also setting cpus.
+            Note: This value has no effect without also setting `cpus`.
         nodes (int):
             Search for Jobs with exactly this many nodes.
-            Note: If you also specify max_nodes, then this value will act as
+            Note: If you also specify `max_nodes`, then this value will act as
             the minimum.
         max_nodes (int):
             Search for Jobs with no more than this amount of nodes.
-            Note: This value has no effect without also setting nodes.
-        qos (list):
+            Note: This value has no effect without also setting `nodes`.
+        qos (list[str]):
             Search for Jobs with these Qualities of Service.
-        names (list):
+        names (list[str]):
             Search for Jobs with these job names.
-        partitions (list):
+        partitions (list[str]):
             Search for Jobs with these partition names.
-        groups (list):
-            Search for Jobs with these group names. You can both specify the
-            groups as string or by their GID.
+        groups (list[str]):
+            Search for Jobs with these group names. Alternatively, you can
+            also specify the GIDs directly.
         timelimit (Union[str, int]):
             Search for Jobs with exactly this timelimit.
-            Note: If you also specify max_timelimit, then this value will act
+            Note: If you also specify `max_timelimit`, then this value will act
             as the minimum.
         max_timelimit (Union[str, int]):
             Search for Jobs which run no longer than this timelimit
-            Note: This value has no effect without also setting timelimit
-        users (list):
-            Search for Jobs with these user names. You can both specify the
-            users as string or by their UID.
-        wckeys (list):
+            Note: This value has no effect without also setting `timelimit`
+        users (list[str]):
+            Search for Jobs with these user names. Alternatively, you can also
+            specify the UIDs directly.
+        wckeys (list[str]):
             Search for Jobs with these WCKeys
-        nodelist (list):
+        nodelist (list[str]):
             Search for Jobs that ran on any of these Nodes
         with_script (bool):
             Instruct the slurmdbd to also send the job script(s)
             Note: This requires specifying explictiy job ids, and is mutually
-            exclusive with with_env
+            exclusive with `with_env`
         with_env (bool):
             Instruct the slurmdbd to also send the job environment(s)
             Note: This requires specifying explictiy job ids, and is mutually
-            exclusive with with_script
+            exclusive with `with_script`
     """
     cdef slurmdb_job_cond_t *ptr
 

pyslurm/db/job.pyx

@@ -87,28 +87,12 @@ cdef class JobSearchFilter:
     def _parse_groups(self):
         if not self.groups:
             return None
-
-        gid_list = []
-        for group in self.groups:
-            if isinstance(group, int):
-                gid_list.append(group)
-            else:
-                gid_list.append(group_to_gid(group))
-
-        return gid_list
+        return list({group_to_gid(group) for group in self.groups})
 
     def _parse_users(self):
         if not self.users:
             return None
-
-        uid_list = []
-        for user in self.users:
-            if not isinstance(user, list):
-                uid_list.append(int(user))
-            elif user:
-                uid_list.append(user_to_uid(user))
-
-        return uid_list
+        return list({user_to_uid(user) for user in self.users})
 
     def _parse_clusters(self):
         if not self.clusters:

pyslurm/utils/helpers.pyx

@@ -95,10 +95,10 @@ def user_to_uid(user, err_on_invalid=True):
         return slurm.NO_VAL
 
     try:
-        if isinstance(user, str):
+        if isinstance(user, str) and not user.isdigit():
             return getpwnam(user).pw_uid
 
-        return getpwuid(user).pw_uid
+        return getpwuid(int(user)).pw_uid
     except KeyError as e:
         if err_on_invalid:
             raise e
@@ -112,10 +112,10 @@ def group_to_gid(group, err_on_invalid=True):
         return slurm.NO_VAL
 
     try:
-        if isinstance(group, str):
+        if isinstance(group, str) and not group.isdigit():
             return getgrnam(group).gr_gid
 
-        return getgrgid(group).gr_gid
+        return getgrgid(int(group)).gr_gid
     except KeyError as e:
         if err_on_invalid:
             raise e

tests/unit/test_common.py

@@ -309,8 +309,9 @@ def test_parse_uid(self):
         name = uid_to_name(0, lookup=lookup) 
         assert name == "root"
 
-        uid = user_to_uid("root")
-        assert uid == 0
+        assert user_to_uid("root") == 0
+        assert user_to_uid(0) == 0
+        assert user_to_uid("0") == 0
 
         with pytest.raises(KeyError):
             name = uid_to_name(2**32-5)
@@ -326,8 +327,9 @@ def test_parse_gid(self):
         name = gid_to_name(0, lookup=lookup) 
         assert name == "root"
 
-        gid = group_to_gid("root")
-        assert gid == 0
+        assert group_to_gid("root") == 0
+        assert group_to_gid(0) == 0
+        assert group_to_gid("0") == 0
 
         with pytest.raises(KeyError):
             name = gid_to_name(2**32-5)

tests/unit/test_db_job.py

@@ -24,7 +24,7 @@
 import pyslurm
 
 
-def test_search_filter():
+def test_filter():
     job_filter = pyslurm.db.JobSearchFilter()
 
     job_filter.clusters = ["test1"]