From 53809d7c23cc8569e448c289b5664fc1422ffd69 Mon Sep 17 00:00:00 2001 From: Giovanni Torres Date: Thu, 20 Aug 2026 10:32:11 -0400 Subject: [PATCH] fix: fix json serialization for enum types --- CHANGELOG.md | 11 +++ pyslurm/core/reservation.pxd | 4 +- pyslurm/core/reservation.pyx | 13 ++- pyslurm/utils/enums.pyx | 29 +++++- pyslurm/utils/helpers.pyx | 16 +++- tests/integration/test_reservation.py | 25 ++++++ tests/unit/test_reservation.py | 122 ++++++++++++++++++++++++++ 7 files changed, 211 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2262a2b5..f8d22b1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `reserved_ports` - `extra` +### Fixed + +- `pyslurm.Reservations.to_json()` raised `TypeError: Object of type ReservationFlags is not JSON serializable`. + Calling `to_dict(recursive=True)` now converts Flags to a `list` of `str` and Enums to their plain value. +- Setting `flags` on `pyslurm.Reservation` from a `list` of `pyslurm.ReservationFlags` members silently set no + Flag at all. Members and (case-insensitive) names are now both accepted, and an unknown name raises a + `ValueError` instead of being ignored. +- Loading a Reservation with a Flag that pyslurm does not have a member for no longer raises a `ValueError`. + Unknown bits are now ignored. + ### Deprecated - The `gres_per_node` attribute of `pyslurm.Job` will soon be removed. Use `gres` instead. @@ -89,6 +99,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `pyslurm.db.TrackableResources` no longer inherits from `dict`, and now has properly all possible TRES in Slurm defined. - Type for `tres_per_task` in `pyslurm.Job` has been changed to `pyslurm.db.TrackableResources` +- New Flags added to `pyslurm.ReservationFlags`: `REPLACE`, `TIME_FLOAT` and `FORCE_START` ## [25.11.0](https://github.com/PySlurm/pyslurm/releases/tag/v25.11.0) - 2026-02-13 diff --git a/pyslurm/core/reservation.pxd b/pyslurm/core/reservation.pxd index e5b9e62a..781c6e5a 100644 --- a/pyslurm/core/reservation.pxd +++ b/pyslurm/core/reservation.pxd @@ -147,7 +147,9 @@ cdef class Reservation: flags = ["MAINTENANCE", "FLEX", "MAGNETIC"] When setting like this, the strings must match the names of members - in [pyslurm.ReservationFlags][]. + in [pyslurm.ReservationFlags][], otherwise a [ValueError][] is + raised. Members of [pyslurm.ReservationFlags][] are accepted in the + list as well. reoccurrence (pyslurm.ReservationReoccurrence): Describes if and when this Reservation reoccurs. Since [pyslurm.ReservationReoccurrence] members are also just diff --git a/pyslurm/core/reservation.pyx b/pyslurm/core/reservation.pyx index aeafd082..26916932 100644 --- a/pyslurm/core/reservation.pyx +++ b/pyslurm/core/reservation.pyx @@ -180,6 +180,14 @@ cdef class Reservation: def to_dict(self, recursive = False): """Reservation information formatted as a dictionary. + Args: + recursive (bool, optional): + When this is set to `True`, everything is converted into plain, + JSON-serializable data. `flags` becomes a [list][] of [str][] + and `reoccurrence` a plain [str][]. Default is `False`, which + keeps [pyslurm.ReservationFlags][] and + [pyslurm.ReservationReoccurrence][] objects. + Returns: (dict): Reservation information as dict @@ -502,7 +510,7 @@ cdef class Reservation: @property def flags(self): - return ReservationFlags(self.info.flags) + return ReservationFlags.from_int(self.info.flags) @flags.setter def flags(self, val): @@ -534,7 +542,10 @@ class ReservationFlags(SlurmFlag): ALL_NODES = slurm.RESERVE_FLAG_ALL_NODES SKIP = slurm.RESERVE_FLAG_SKIP SCHED_FAILED = slurm.RESERVE_FLAG_SCHED_FAILED + REPLACE = slurm.RESERVE_FLAG_REPLACE REPLACE_DOWN = slurm.RESERVE_FLAG_REPLACE_DOWN + TIME_FLOAT = slurm.RESERVE_FLAG_TIME_FLOAT + FORCE_START = slurm.RESERVE_FLAG_FORCE_START GRES_REQUIRED = slurm.RESERVE_FLAG_GRES_REQ TRES_PER_NODE = slurm.RESERVE_TRES_PER_NODE diff --git a/pyslurm/utils/enums.pyx b/pyslurm/utils/enums.pyx index 5eb07a79..55b86662 100644 --- a/pyslurm/utils/enums.pyx +++ b/pyslurm/utils/enums.pyx @@ -100,15 +100,38 @@ class SlurmFlag(Flag, metaclass=DocstringSupport): obj._clear_flag = int(args[0]) if len(args) >= 1 else 0 return obj + @classmethod + def from_int(cls, value): + # Any bit that has no member here is silently dropped, so that Flags + # a newer Slurm version may set do not break decoding. + known = 0 + for flag in cls: + known |= flag.value + + return cls(int(value) & known) + @classmethod def from_list(cls, inp): out = cls(0) - for flag in cls: - if flag.name in inp: - out |= flag + for item in inp: + if isinstance(item, cls): + out |= item + continue + + try: + out |= cls[str(item).upper()] + except KeyError: + raise ValueError( + f"Invalid {cls.__name__}: {item}. Possible values are: " + f"{[flag.name for flag in cls]}" + ) from None return out + def to_list(self): + """Names of all the Flags that are set, as a list of strings.""" + return [flag.name for flag in self.__class__ if flag in self] + def _get_flags_cleared(self): val = self.value for flag in self.__class__: diff --git a/pyslurm/utils/helpers.pyx b/pyslurm/utils/helpers.pyx index 99cc895a..f80de98b 100644 --- a/pyslurm/utils/helpers.pyx +++ b/pyslurm/utils/helpers.pyx @@ -27,6 +27,7 @@ from grp import getgrgid, getgrnam, getgrall from pwd import getpwuid, getpwnam, getpwall from os import getuid, getgid from itertools import chain +from enum import Enum, Flag import re import signal from pyslurm.constants import UNLIMITED @@ -333,13 +334,20 @@ def instance_to_dict(inst, recursive=False): cdef dict out = {} for attr in dir(inst): val = getattr(inst, attr) - private_attr = attr.startswith("_") - if not private_attr and recursive and hasattr(val, "to_dict"): - val = val.to_dict(recursive=recursive) - elif private_attr or callable(val): + if attr.startswith("_") or callable(val): continue + if recursive: + # Convert everything into plain, JSON-serializable data. + if hasattr(val, "to_dict"): + val = val.to_dict(recursive=recursive) + elif isinstance(val, Flag): + # Flag must be checked before Enum, since it is a subclass. + val = val.to_list() + elif isinstance(val, Enum): + val = val.value + out[attr] = val return out diff --git a/tests/integration/test_reservation.py b/tests/integration/test_reservation.py index 610cd7cd..7462d6e9 100644 --- a/tests/integration/test_reservation.py +++ b/tests/integration/test_reservation.py @@ -20,6 +20,8 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. """test_reservation.py - integration test reservation functionalities.""" +import json + import pyslurm from pyslurm import ReservationFlags, ReservationReoccurrence from datetime import datetime @@ -93,3 +95,26 @@ def test_api_calls(): resv.delete() reservations = pyslurm.Reservations.load() assert len(reservations) == 0 + + +def test_to_json(): + resv = pyslurm.Reservation( + name="testing_json", + start_time=datetime.now(), + duration="1-00:00:00", + users=["root"], + node_count=1, + flags=[ReservationFlags.MAINTENANCE], + ) + resv.create() + + try: + reservations = pyslurm.Reservations.load() + json_data = reservations.to_json() + dict_data = json.loads(json_data) + + assert dict_data + assert dict_data["testing_json"]["flags"] == ["MAINTENANCE"] + assert dict_data["testing_json"]["reoccurrence"] == "NO" + finally: + resv.delete() diff --git a/tests/unit/test_reservation.py b/tests/unit/test_reservation.py index 75ad5f26..df7a9a80 100644 --- a/tests/unit/test_reservation.py +++ b/tests/unit/test_reservation.py @@ -20,9 +20,13 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. """test_reservation.py - Unit test basic reservation functionalities.""" +import json +import pytest + import pyslurm from pyslurm import ReservationFlags, ReservationReoccurrence from datetime import datetime +from enum import Enum def test_create_instance(): @@ -108,3 +112,121 @@ def test_flags_gres_tres(): assert ReservationFlags.TRES_PER_NODE in decoded assert ReservationFlags.SPECIFIC_NODES in decoded + +def test_flags_to_list(): + flags = ReservationFlags.MAINTENANCE | ReservationFlags.FLEX + assert flags.to_list() == ["MAINTENANCE", "FLEX"] + + +def test_flags_to_list_when_no_flags_are_set(): + assert ReservationFlags(0).to_list() == [] + + +def test_to_dict_recursive_converts_flags_to_list(): + resv = pyslurm.Reservation("test") + resv.flags = ReservationFlags.MAINTENANCE | ReservationFlags.FLEX + + data = resv.to_dict(recursive=True) + assert data["flags"] == ["MAINTENANCE", "FLEX"] + + # Without recursive, the rich Flag object is kept. + assert resv.to_dict()["flags"] == ( + ReservationFlags.MAINTENANCE | ReservationFlags.FLEX + ) + + +def test_to_dict_recursive_converts_reoccurrence_to_str(): + resv = pyslurm.Reservation("test") + resv.reoccurrence = ReservationReoccurrence.DAILY + + data = resv.to_dict(recursive=True) + assert data["reoccurrence"] == "DAILY" + assert not isinstance(data["reoccurrence"], Enum) + + +def test_to_dict_recursive_is_json_serializable(): + resv = pyslurm.Reservation("test") + resv.flags = ReservationFlags.ANY_NODES + + data = json.loads(json.dumps(resv.to_dict(recursive=True))) + assert data["flags"] == ["ANY_NODES"] + assert data["reoccurrence"] == "NO" + + +def test_to_dict_recursive_is_json_serializable_without_flags(): + # Every Reservation has a `flags` attribute, so serializing used to fail + # even when no flag at all is set. + resv = pyslurm.Reservation("test") + + data = json.loads(json.dumps(resv.to_dict(recursive=True))) + assert data["flags"] == [] + + +def test_flags_time_float_replace_and_force_start(): + # Regression: these flags are set by Slurm on reservations created with + # e.g. `flags=TIME_FLOAT`, but had no member here, which made decoding + # such a reservation raise ValueError. + combo = ( + ReservationFlags.TIME_FLOAT + | ReservationFlags.REPLACE + | ReservationFlags.FORCE_START + ) + decoded = ReservationFlags(combo.value) + assert ReservationFlags.TIME_FLOAT in decoded + assert ReservationFlags.REPLACE in decoded + assert ReservationFlags.FORCE_START in decoded + + +def test_flags_from_int_ignores_unknown_bits(): + # Bits that a newer Slurm may set, but that pyslurm does not know about + # yet, must not make decoding fail. + value = ReservationFlags.MAINTENANCE.value | (1 << 63) + assert ReservationFlags.from_int(value) == ReservationFlags.MAINTENANCE + + +def test_flags_from_int_with_known_bits(): + flags = ReservationFlags.MAINTENANCE | ReservationFlags.FLEX + assert ReservationFlags.from_int(flags.value) == flags + + +def test_flags_from_int_without_any_bits(): + assert ReservationFlags.from_int(0) == ReservationFlags(0) + + +def test_flags_from_list_with_enum_members(): + flags = ReservationFlags.from_list( + [ReservationFlags.MAINTENANCE, ReservationFlags.FLEX] + ) + assert flags == ReservationFlags.MAINTENANCE | ReservationFlags.FLEX + + +def test_flags_from_list_is_case_insensitive(): + assert ReservationFlags.from_list(["flex"]) == ReservationFlags.FLEX + + +def test_flags_from_list_with_unknown_flag(): + with pytest.raises(ValueError, match="NOT_A_FLAG"): + ReservationFlags.from_list(["NOT_A_FLAG"]) + + +def test_flags_from_list_roundtrip(): + flags = ReservationFlags.MAINTENANCE | ReservationFlags.ANY_NODES + assert ReservationFlags.from_list(flags.to_list()) == flags + + +def test_flags_setter_with_enum_member_list(): + resv = pyslurm.Reservation("test") + resv.flags = [ReservationFlags.MAINTENANCE, "FLEX"] + assert resv.flags == ReservationFlags.MAINTENANCE | ReservationFlags.FLEX + + +def test_collection_to_json(): + resv = pyslurm.Reservation("test") + resv.flags = ReservationFlags.ANY_NODES + + reservations = pyslurm.Reservations() + reservations.add(resv) + + data = json.loads(reservations.to_json()) + assert data["test"]["flags"] == ["ANY_NODES"] +