Un special-case system drive btrfs-in-partition treatment #2824 #2825
Conversation
Includes: - Modernisation re type hints in scan_disks(). - Programmatically establish `lsblk` field derived Disk namedtuple fields. - Refactoring for readability. - Black reformatting.
|
@Hooverdan96 Thanks for the review: but this is very much a draft still: I've not actually begun the functional changes. And I'll have to squash these and many other changes to come so your contributions/attribution will be lost. But thanks for the typos anyway. I'll incorporate when next working on this draft: soon hopefully. Pushing my work as I go here though just in case - and easier to ask advice etc if I get stuck. Or do end-to-end testing via our back-end rpmbuild against PRs (draft or otherwise). I think I have a few simplifications on the way, and hopefully the end result will be more maintainable. Pretty sure we can simplify quite a bit: as what we have currently is definitely super stretched. I also plan to make more use of our Disk namedtuple in scan_disks() so the code can be more self documenting, and to help guard against accidental changes we don't intend: named tuples are immutable. |
|
Understood absolutely, and I'm not craving attribution for those cosmetic things, but figured if I mention them now, you can get rid of that, especially since you will now start focusing on making actual functional changes... Godspeed on the simplification, and definitely makes sense to me if you will be able to put a more complete dataset/logic on the |
Replace by-hand lsblk output line parser with dictionary comprehension. Successive work towards modernisation/simplification of scan_disks(). Employ defaults in Disk named tuple to ease creation using lsblk info, as these defaults pertain to our extra flag info.
By normalising on lowecase we can ease the transition to using our Disk named tuple as the working copy: to help readability and highlight where we write (via Disk.replace(root=True)). This should also simplify the build of our final return value. We also have a large number of tests that use the existing lowercase Disk fields.
Explicit, rather than programmatic, namedtuple difintion works way better for PyCharm editor assistance. Otherwise it fails through to generic tuple hints/fencing.
By working directly with the Disk namedtuple we gain readability and the advantages of an immutable type: i.e. more explicit where writes/changes happen due to requirement to use ._replace(). We also require less type transitions.
|
At this point in this draft PR associated branch all existing osi.py tests again pass. And we have maintained the existing observed failure in the 2 new tests associated with draft PR: "Disk being misattributed to ROOT pool #2749" #2823. So we are still at no functional change: as intended. But the code is now likely more robust and easier to read with many more type hints and way more obvious re internal changes and intent within scan_disks(). Next:
|
phillxnet
deleted the
2824-Un-special-case-system-drive-btrfs-in-partition-treatment
branch
|
This Draft PR's work-to-date represents a significant enough modification to address a stepping stone defined by the following issue: Modernise scan_disks() - no functional change intended #2826 for which a new PR will be presented shorlty. |
No functional change on first commit. Some basic code improvements to clear the way for the planed simplification/modification later in this draft PR.
Includes:
lsblkfield derived Disk namedtuple fields.See issue text #2824