Add option to pretty print the JSON Output #34
Conversation
|
Splitout of #31 |
Codecov Report
@@ Coverage Diff @@
## main #34 +/- ##
==========================================
- Coverage 51.60% 50.51% -1.09%
==========================================
Files 4 4
Lines 655 675 +20
==========================================
+ Hits 338 341 +3
- Misses 317 334 +17
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| apt = 4 | ||
| """ | ||
| A repository which is managed by apt. | ||
| """ | ||
|
|
||
|
|
||
| class Osversion: |
There was a problem hiding this comment.
If you ask me, this class really wants to be a dataclass. I wonder how much mutability you need, are the deleters and setters really needed?
I could imagine something like the below is working as well, but I don't have the full context of theses models:
@dataclasses.dataclass
class OSVersion: # pylint: disable = too-many-instance-attributes
"""Version of an OS breed."""
signatures: list
version_file: str
version_file_regex: str
kernel_arch: str
kernel_arch_regex: str
supported_arches: list
supported_repo_breeds: list
kernel_file: str
initrd_file: str
isolinux_ok: bool
default_autoinstall: str
kernel_options: str
kernel_options_post: str
template_files: str
boot_files: list
boot_loaders: dict
def encode(self) -> dict:
"""
Encodes a single :class:`Osversion`. This means that the properties of an object is transferred into a JSON.
:return: The dictionary with the data.
"""
return dataclasses.asdict(self)
# instead of
version = Osversion()
version.decode(my_dict)
# you'd use
version = OSVersion(**my_dict)The upside of using dataclasses is that you only need to write what attributes you have and what type they should have and the rest is autogenerated. But you'd lose strict type checking at runtime, at least without additional code/libraries.
There was a problem hiding this comment.
Well what is missing currently is the fact that for example I want to do more checks in the setters. For example I do want to do regex validation and give back the actual objects on the getters later. Also I don't want consumers to be able to acutally delete attributes, thus the explicit overwrite of the deleters. Also this is a nice solution in my eyes to have a possibility to reset values to their defaults if required.
Tbh. I didn't knew the concept of a dataclass before but I think especially for the dict and list the validation should be there at least which then means that a dataclass is not enough. What I like is that the objects can be made immutable but since I am using lists and dicts that sadly is not possible to my understanding.
If I only would limit myself to type checking I found this library - https://pypi.org/project/dataclass-type-validator/ - but I am not sure if it is wise to do only minimal validation if you have user input... My design skills are sadly not my biggest strength.
There was a problem hiding this comment.
Summary of a talk @agraul @meaksh and I had after our standup: Since the only consumers of this library are the CLI and Cobbler itself, currently and in the foreseeable future, it makes more sense to handle the data validation directly in these points. This point is strengthened by the fact that it doesn't matter how much validation I will build in because in Python there is always around my code. We also discussed other solutions but came to the conclusion that @agraul's solution with the dataclasses is the most elegant for now.
Remove the imports and apply @meaksh s suggestions
SchoolGuy
force-pushed
the
feature/pretty-print
branch
from
c8cf3a5 to
fbe942f
Compare
|
Rebased since PR #33 was merged |
This should be rebased after PR #33 and then reviewed.