Add Python bindings for libnetplan#385
Conversation
slyon
force-pushed
the
bindings
branch
9 times, most recently
from
fffd856 to
b76bfdf
Compare
slyon
force-pushed
the
bindings
branch
20 times, most recently
from
8249583 to
e646fe5
Compare
slyon
force-pushed
the
bindings
branch
9 times, most recently
from
04e35fe to
8fd5adf
Compare
daniloegea
left a comment
daniloegea
left a comment
There was a problem hiding this comment.
Nice work! Implementing bindings with cffi is much nicer than ctypes as far as could see.
In my opinion this could be merged. I just added a few non-blocking comments. But maybe you'd like to address some of them, such as a deprecation warning from meson and including veths and dummies in the netdef class.
| if not next_value: | ||
| raise StopIteration | ||
| content = next_value | ||
| # XXX: Introduce getters for .address/.lifetime/.label, to avoid |
There was a problem hiding this comment.
thought: hmm it will be even worse with routes...
There was a problem hiding this comment.
Yes.. we should build proper API around it, eventually. And keep it internal until that's done.
And make the CI tests pass, using the new 'netplan' Python module bindigns.
|
Thanks @daniloegea! I've rebased and addressed most of your remarks. I still need to improve my comments/description in |
daniloegea
left a comment
daniloegea
left a comment
There was a problem hiding this comment.
Looks good!
Only one thing I'd like to be changed before merging: the new gcovr.cfg file probably should use the filter configuration to include the source code instead of exclude with a hardcoded build directory. The test coverage-c will fail if you use a different build directory.
|
Thanks, I've addressed all of your comments. CI failures about Azure kernels are unrelated (a basic local autopkgtest passed): Let's merge this! |
2 participants
Description
Implementation of public Python bindings for
libnetplan.so, using CFFI in API mode, as described in spec FO0114. The diff is fairly big, but it contains lots of refactorings, effectively moving 700 LOC of our old/internal ctypes-bindings over into a public module and porting them to make use of CFFI. Going through the commits one by one should make the review manageable:8184c2b parse-nm: improve some error passing
59ee33c CI: Add python3-cffi build-dependency
032e356 include: Extend public API functions
9a84892 src: make internal API functions/structures available
4bfe29b bindings: python-cffi build skeleton
fdad4e0 bindings: Initial implementation of CFFI Python bindings
40c4f47 cli:tools:test: Make use of new cffi python bindings
a21d9f4 cli: drop legacy/internal ctypes bindings, now unused
8fd5adf examples: Add an example of how to use Netplan's CFFI bindings
The most interesting bits are porbably those commits, where the new module is introduced, to be installed into
usr/lib/python3/dist-packages/netplan/:We're using the file in
python-cffi/netplan/_build_cffi.pyto build a_netplan_cffi.cpython-311-x86_64-gnu.so(internal) binary module, that is linked tolibnetplan.soitself. In theffibuilder.cdef(...)we declare any functions/constants/structures that we want to make available from libnetplan.so to the Python bindings. The compiler will match those against Netplan's headers at build time. Inpython-cffi/netplan/{parser,state,netdef}.pywe're then making use of those declarations to define our high-level, Python-native bindings.We're still making some limited use of ctypes in our tests (
test_libnetplan.py) in order to validate some raw library functionality, which are not necessarily exposed through the Python bindings.Checklist
make checksuccessfully.make check-coverage).in Launchpad: FR-2780