From db0349e6361c81efba63d747ca803533703304a7 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 29 Apr 2026 09:29:55 +0200 Subject: [PATCH 1/8] maintainership: Provide default for the document field. The field is not optional, and requires a default to be able to correctly instantiate the model without additional parameters. --- osc/gitea_api/maintainership.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osc/gitea_api/maintainership.py b/osc/gitea_api/maintainership.py index 404dc43d3..f999e1c40 100644 --- a/osc/gitea_api/maintainership.py +++ b/osc/gitea_api/maintainership.py @@ -19,7 +19,7 @@ class MaintainershipHeader(BaseModel): """ A model representing the maintainership document header. """ - document: MaintainershipDocumentType = Field() + document: MaintainershipDocumentType = Field(default="obs-maintainers") version: str = Field(default="1.0") From 3564d8435426feff904ce90db634b00d500263d0 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 29 Apr 2026 09:33:32 +0200 Subject: [PATCH 2/8] models: Stop deriving FromParent from NotSetClass This is fairly impractical, and there is no real reason. --- osc/util/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osc/util/models.py b/osc/util/models.py index d6deb3211..4748beb16 100644 --- a/osc/util/models.py +++ b/osc/util/models.py @@ -82,7 +82,7 @@ def __bool__(self): NotSet = NotSetClass() -class FromParent(NotSetClass): +class FromParent: def __init__(self, field_name, *, fallback=NotSet): self.field_name = field_name self.fallback = fallback From 03d3660807edc37767753b55f9cc415e92acb559 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 29 Apr 2026 09:41:48 +0200 Subject: [PATCH 3/8] models: Fix NotSet check The NotSet object is not a singleton, it is required to check the class. --- osc/util/models.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osc/util/models.py b/osc/util/models.py index 4748beb16..ea4c83e06 100644 --- a/osc/util/models.py +++ b/osc/util/models.py @@ -123,7 +123,7 @@ def __init__( # append information about the default value if isinstance(self.default, FromParent): self.__doc__ += f"\n\nDefault: inherited from parent config's field ``{self.default.field_name}``" - elif self.default is not NotSet: + elif not isinstance(self.default, NotSetClass): self.__doc__ += f"\n\nDefault: ``{self.default}``" # whether to exclude this field from export @@ -308,13 +308,13 @@ def get(self, obj): if isinstance(self.default, FromParent): if obj._parent is None: - if self.default.fallback is not NotSet: + if not isinstance(self.default.fallback, NotSetClass): return self.default.fallback else: raise RuntimeError(f"The field '{self.name}' has default {self.default} but the model has no parent set") return getattr(obj._parent, self.default.field_name or self.name) - if self.default is NotSet: + if isinstance(self.default, NotSetClass): raise RuntimeError(f"The field '{self.name}' has no default") # make a deepcopy to avoid problems with mutable defaults @@ -397,7 +397,7 @@ def __new__(mcs, name, bases, attrs): field.get_copy.__func__.__annotations__ = {"return": field.type} # set 'None' as the default for optional fields - if field.default is NotSet and field.is_optional: + if isinstance(field.default, NotSetClass) and field.is_optional: field.default = None return new_cls @@ -424,7 +424,7 @@ def __init__(self, **kwargs): for name, field in self.__fields__.items(): if name not in kwargs: - if field.default is NotSet: + if isinstance(field.default, NotSetClass): uninitialized_fields.append(field.name) continue value = kwargs.pop(name) From 5e7a6bf8140723f0c09167a5e6abea1786cf49de Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 29 Apr 2026 09:51:48 +0200 Subject: [PATCH 4/8] maintainership: Drop useless None default The default for users and groups is None which is not useful in any way. --- osc/gitea_api/maintainership.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osc/gitea_api/maintainership.py b/osc/gitea_api/maintainership.py index f999e1c40..b28addc07 100644 --- a/osc/gitea_api/maintainership.py +++ b/osc/gitea_api/maintainership.py @@ -7,8 +7,8 @@ class MaintainerInfo(BaseModel): """ A model representing users and groups associated with a project or package. """ - users: Optional[List[str]] = Field(default=None) - groups: Optional[List[str]] = Field(default=None) + users: Optional[List[str]] = Field() + groups: Optional[List[str]] = Field() class MaintainershipDocumentType(str, Enum): From 0cc253086e68b5293783e07aff5a0b6e9f87686d Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 29 Apr 2026 09:59:19 +0200 Subject: [PATCH 5/8] models: Allow optional fields to be unset --- osc/util/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osc/util/models.py b/osc/util/models.py index ea4c83e06..0fd74f6f8 100644 --- a/osc/util/models.py +++ b/osc/util/models.py @@ -424,7 +424,7 @@ def __init__(self, **kwargs): for name, field in self.__fields__.items(): if name not in kwargs: - if isinstance(field.default, NotSetClass): + if isinstance(field.default, NotSetClass) and not field.is_optional: uninitialized_fields.append(field.name) continue value = kwargs.pop(name) From 2a688dd5b246acd9209a970f1bd0857ba8e65f16 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 29 Apr 2026 10:10:37 +0200 Subject: [PATCH 6/8] models: Skip fields set to None in dict The Optional constructor is union with None. As a result optional fields are set to None when not present. Ignore them in dict(). --- osc/gitea_api/maintainership.py | 2 +- osc/util/models.py | 14 +++++++------- tests/test_models.py | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/osc/gitea_api/maintainership.py b/osc/gitea_api/maintainership.py index b28addc07..ad624a1c0 100644 --- a/osc/gitea_api/maintainership.py +++ b/osc/gitea_api/maintainership.py @@ -82,7 +82,7 @@ def to_string(self): - sort keys - indent by 2 spaces """ - return super().to_string(exclude_none=True, sort_keys=True, indent=2) + return super().to_string(sort_keys=True, indent=2) def get_package_maintainers_users(self, package: str) -> List[str]: if package not in self.packages: diff --git a/osc/util/models.py b/osc/util/models.py index 0fd74f6f8..3aedcd78c 100644 --- a/osc/util/models.py +++ b/osc/util/models.py @@ -469,20 +469,20 @@ def __lt__(self, other): return False return self._get_cmp_data() < other._get_cmp_data() - def dict(self, *, exclude_none: bool = False): + def dict(self): result = {} for name, field in self.__fields__.items(): if field.exclude: continue value = getattr(self, name) - if exclude_none and value is None: + if value is None: continue if value is not None and field.is_model: - result[name] = value.dict(exclude_none=exclude_none) + result[name] = value.dict() elif value is not None and field.is_model_list: - result[name] = [i.dict(exclude_none=exclude_none) for i in value] + result[name] = [i.dict() for i in value] elif value is not None and field.is_model_dict: - result[name] = {k: v.dict(exclude_none=exclude_none) for k, v in value.items()} + result[name] = {k: v.dict() for k, v in value.items()} else: result[name] = value @@ -522,13 +522,13 @@ def from_string(cls, text: str) -> "Self": obj = cls(**data) return obj - def to_string(self, *, exclude_none: bool = False, sort_keys: bool = False, indent: int = 4) -> str: + def to_string(self, *, sort_keys: bool = False, indent: int = 4) -> str: """ Dump model to a json string. """ import json - result = json.dumps(self.dict(exclude_none=exclude_none), sort_keys=sort_keys, indent=indent) + result = json.dumps(self.dict(), sort_keys=sort_keys, indent=indent) return result def do_snapshot(self): diff --git a/tests/test_models.py b/tests/test_models.py index 0bac91a79..23cff45c0 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -31,7 +31,7 @@ class TestModel(BaseModel): text: str | None = Field() m = TestModel() - self.assertEqual(m.dict(), {"text": None}) + self.assertEqual(m.dict(), {}) self.assertRaises(TypeError, setattr, m.text, 123) @@ -45,7 +45,7 @@ class TestModel(BaseModel): sub: Optional[List[TestSubmodel]] = Field(default=None) m = TestModel() - self.assertEqual(m.dict(), {"a": "default", "b": None, "sub": None}) + self.assertEqual(m.dict(), {"a": "default"}) m.b = "B" m.sub = [{"text": "one"}, {"text": "two"}] From 7ee1395ee734b22f0221fce2d31f3221dad2a443 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Thu, 30 Apr 2026 18:57:35 +0200 Subject: [PATCH 7/8] models: Remove option for producing non-standard JSON formatting There is no user that requires this bespoke format --- osc/gitea_api/maintainership.py | 11 ----------- osc/util/models.py | 13 +++++++++---- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/osc/gitea_api/maintainership.py b/osc/gitea_api/maintainership.py index ad624a1c0..6d3344c7d 100644 --- a/osc/gitea_api/maintainership.py +++ b/osc/gitea_api/maintainership.py @@ -73,17 +73,6 @@ def from_string(cls, text: str) -> "Maintainership": return cls(**data) - def to_string(self): - """ - Export _maintainership.json contents. - - We always: - - exclude entries that have empty (None) value - - sort keys - - indent by 2 spaces - """ - return super().to_string(sort_keys=True, indent=2) - def get_package_maintainers_users(self, package: str) -> List[str]: if package not in self.packages: raise ValueError(f"Package '{package}' not found in maintainership data.") diff --git a/osc/util/models.py b/osc/util/models.py index 3aedcd78c..a6a818c02 100644 --- a/osc/util/models.py +++ b/osc/util/models.py @@ -508,8 +508,8 @@ def to_file(self, path: str): import json with open(path, "w", encoding="utf-8") as f: - # we prefer key ordering according to the fields in the model - json.dump(self.dict(), f, sort_keys=False, indent=4) + # we prefer fixed, well-defined key ordering + json.dump(self.dict(), f, sort_keys=True, indent=2) @classmethod def from_string(cls, text: str) -> "Self": @@ -522,13 +522,18 @@ def from_string(cls, text: str) -> "Self": obj = cls(**data) return obj - def to_string(self, *, sort_keys: bool = False, indent: int = 4) -> str: + def to_string(self) -> str: """ Dump model to a json string. + + We always: + - exclude entries that have empty (None) value + - sort keys + - indent by 2 spaces """ import json - result = json.dumps(self.dict(), sort_keys=sort_keys, indent=indent) + result = json.dumps(self.dict(), sort_keys=True, indent=2) return result def do_snapshot(self): From 0112c624cc03495346c3bb893f130b251cb306a2 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Thu, 30 Apr 2026 14:13:10 +0200 Subject: [PATCH 8/8] models: Remove NotSet value As NotSet and None is used interchangeably 'for usability' NotSet can as well be abolished, it only makes the code more confusing. There is no representation of None in XML, and no protocol that uses JSON requires a null value, there is no use for NotSet separate from None. If an actual NotSet value separate from None is needed it should be implemented correctly, so that it is usable without substituting with None. --- osc/util/models.py | 29 ++++++----------------------- tests/test_models.py | 8 -------- 2 files changed, 6 insertions(+), 31 deletions(-) diff --git a/osc/util/models.py b/osc/util/models.py index a6a818c02..ed1f9caf2 100644 --- a/osc/util/models.py +++ b/osc/util/models.py @@ -58,7 +58,6 @@ class UnionType: "BaseModel", "XmlModel", "Field", - "NotSet", "FromParent", "Enum", "Dict", @@ -71,19 +70,8 @@ class UnionType: ) -class NotSetClass: - def __repr__(self): - return "NotSet" - - def __bool__(self): - return False - - -NotSet = NotSetClass() - - class FromParent: - def __init__(self, field_name, *, fallback=NotSet): + def __init__(self, field_name, *, fallback=None): self.field_name = field_name self.fallback = fallback @@ -95,14 +83,13 @@ def __repr__(self): class Field(property, *([Any] if typing.TYPE_CHECKING else [])): def __init__( self, - default: Any = NotSet, + default: Any = None, description: Optional[str] = None, exclude: bool = False, get_callback: Optional[Callable] = None, **extra, ): # the default value; it can be a factory function that is lazily evaluated on the first use - # model sets it to None if it equals to NotSet (for better usability) self.default = default # a flag indicating, whether the default is a callable with lazy evalution @@ -123,7 +110,7 @@ def __init__( # append information about the default value if isinstance(self.default, FromParent): self.__doc__ += f"\n\nDefault: inherited from parent config's field ``{self.default.field_name}``" - elif not isinstance(self.default, NotSetClass): + elif self.default is not None: self.__doc__ += f"\n\nDefault: ``{self.default}``" # whether to exclude this field from export @@ -308,13 +295,13 @@ def get(self, obj): if isinstance(self.default, FromParent): if obj._parent is None: - if not isinstance(self.default.fallback, NotSetClass): + if self.default.fallback is not None or self.is_optional: return self.default.fallback else: raise RuntimeError(f"The field '{self.name}' has default {self.default} but the model has no parent set") return getattr(obj._parent, self.default.field_name or self.name) - if isinstance(self.default, NotSetClass): + if self.default is None and not self.is_optional: raise RuntimeError(f"The field '{self.name}' has no default") # make a deepcopy to avoid problems with mutable defaults @@ -396,10 +383,6 @@ def __new__(mcs, name, bases, attrs): # set annotation for the getter so it shows up in sphinx field.get_copy.__func__.__annotations__ = {"return": field.type} - # set 'None' as the default for optional fields - if isinstance(field.default, NotSetClass) and field.is_optional: - field.default = None - return new_cls @@ -424,7 +407,7 @@ def __init__(self, **kwargs): for name, field in self.__fields__.items(): if name not in kwargs: - if isinstance(field.default, NotSetClass) and not field.is_optional: + if field.default is None and not field.is_optional: uninitialized_fields.append(field.name) continue value = kwargs.pop(name) diff --git a/tests/test_models.py b/tests/test_models.py index 23cff45c0..0d7dc27df 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -16,14 +16,6 @@ def test_get_origin_list_str(self): self.assertEqual(typ, list) -class TestNotSet(unittest.TestCase): - def test_repr(self): - self.assertEqual(repr(NotSet), "NotSet") - - def test_bool(self): - self.assertEqual(bool(NotSet), False) - - class Test(unittest.TestCase): @unittest.skipIf(sys.version_info[:2] < (3, 10), "added in python 3.10") def test_union_or(self):