netplan: set: make it possible to unset a whole devtype subtree (LP: #1942930) (FR-1685)#236
Conversation
This allows the test machinery to do some better error reporting when an exception is raised, as it understands that the issue is an uncaught exception, not a failed comparison.
As I'm expecting this kind of function to be useful for the parser, the implementation is directly in the form of a macro modelled after its counterpart.
schopin-pro
force-pushed
the
lp1942930-unset-tree
branch
4 times, most recently
from
8d821f6 to
7923612
Compare
Codecov Report
@@ Coverage Diff @@
## main #236 +/- ##
=======================================
Coverage 99.05% 99.06%
=======================================
Files 57 57
Lines 9633 9706 +73
=======================================
+ Hits 9542 9615 +73
Misses 91 91
Continue to review full report at Codecov.
|
|
To whoever wants to backport this onto older releases, the first commit can be dropped, the second will mostly likely need some form of rewrite as it builds upon very recent work (#230). AFAICT the last two commits shouldn't pose any problem. |
This is implemented via iterators to limit the knowledge Python has of the underlying memory model. The new symbols are prefixed with '_' to denote that they are not to be used by public consumers. We also expose process_yaml_hierarchy which was already part of the ABI, as it seems needed for the Python helper implementation. Contrary to the usual FFI calls, this one explicitly checks the existence of the symbols and raises an exception if the symbols are not found, as it is not totally unlikely that netplan.io and libnetplan0 be upgraded out of sync. V2: rename the C internal iterator struct, which hadn't evolved along with the successive iterations on the code fixup itar
schopin-pro
force-pushed
the
lp1942930-unset-tree
branch
from
7923612 to
006ee48
Compare
slyon
left a comment
slyon
left a comment
There was a problem hiding this comment.
Thank you Simon, I like how you're using libnetplan's internal data structure here, instead of relying on the deprecated configmanager.py.
The performance isn't good, but still I like that you implemented it in a way that can be backported (relatively) easy, without relying on all the new API code. Clearing a full devtype subtree isn't on the critical path, so this is fine IMO.
+1 from my side! I left two small inline comments, but nothing that would block merging of this PR.
See LP: #1942930 Note that performance for this are horrendous, as we parse the whole tree once for *each* removed netdef! V2: * Lukas as co-author, as he wrote the test! * Clean up commented-out code in the tests Co-authored-by: Lukas Märdian <lukas.maerdian@canonical.com>
schopin-pro
force-pushed
the
lp1942930-unset-tree
branch
from
006ee48 to
c216be9
Compare
3 participants
Description
Previously, trying to unset a whole devtype subtree, such as
netplan set network.ethernets=nullwould fail. This PR fixes it by detecting this special case and manually deleting each netdef assigned to the subtype.This approach is far from being performant, but it has the merit of working without needing too much new code.
Checklist
make checksuccessfully.make check-coverage).