| Message ID | CAA4u48E8791aFihd-uYMMKn9_4pQX2wgHs6fy-3-8gNwP8AXSw@mail.gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
On Tue, Nov 15, 2016 at 12:29 PM, Terry Wilson <twilson@redhat.com> wrote: > The Python library isn't dependent on the code in the OVS tree. It > being in-tree has a few shortcomings. My rationale for recommending > the split: > > * Simple features and bugfixes for the Python lib can't be used by > other projects (like Neutron) until the very latest OVS release is > widely supported > > * Python 3 support is only available in version 2.6.0+ even though the > code would work for previous releases > > * Implying that the Python lib and OVS versions are related when they > aren't is confusing > > * A separate repo would allow adding committers that are familiar with > the Python code, but less familiar with the C code. > > * As a Python-only project, it could be more easily tested, built, and > packaged according to Python project best practices. > > > The current build process requires the Python code, but this is easily > remedied by using git submodules. People tend to reflexively dislike > them, but they are perfect for this application. The OVS tree can lock > to any particular commit for the new python-ovs repo and be guaranteed > that changes don't break the ovs build. I made a fork to show how > simple the change would be: > > https://github.com/otherwiseguy/ovs/commit/6766131b42807829ea78dbc43d164d > b8926030e7 Hi, Terry. Thanks for sharing this proposal. The use of a git submodule seems sane here. It's a nice way to achieve the split without being too invasive to the current build system that makes use of the python code. One thing that would need to be done is a script that creates the new repo from the original code while still retaining all of the git history. For bug fixes, we can request maintenance releases of releases to make those available more quickly. Faster availability of new Python lib features is the biggest benefit of a split to me. Python 3 support was the main case where this would have been helpful, where completion in master and release was about an 8 month gap IIRC. Now that we're on a 6 month release cycle, it shouldn't be that long again, but it can still easily be months. You mention committers, but I don't think there's anything preventing us from adding committers that specialize in the Python code today. You mention testing/building/packaging being easier after the split. Are there particular changes you have in mind? What would be easier? It's also worth nothing that the split does increase project management overhead. It's one more deliverable to release. I don't think it's a big deal, but worth mentioning that there are drawbacks. I'm not aware of any current features in the Python code waiting to be released. I wonder if we should revisit this the next time the issue comes up? If we decide to do it via this approach, it seems like it could be done pretty quickly. > > > commit 6766131b42807829ea78dbc43d164db8926030e7 > Author: Terry Wilson <twilson@redhat.com> > Date: Mon Nov 14 16:03:43 2016 -0600 > > Move python dir to a submodule > > diff --git a/.gitmodules b/.gitmodules > new file mode 100644 > index 0000000..db21481 > --- /dev/null > +++ b/.gitmodules > @@ -0,0 +1,3 @@ > +[submodule "python"] > + path = python > + url = https://github.com/otherwiseguy/python-ovs > diff --git a/Makefile.am b/Makefile.am > index a14d48b..2ce40ee 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -225,7 +225,7 @@ dist-hook-git: distfiles > (cd datapath && $(MAKE) distfiles); > \ > (cat distfiles; sed 's|^|datapath/|' datapath/distfiles) | > \ > LC_ALL=C sort -u > all-distfiles; > \ > - (cd $(srcdir) && git ls-files) | grep -v '\.gitignore$$' | > \ > + (cd $(srcdir) && git ls-files) | grep -v > 'python\|\.git\(ignore\|modules\)$$' | \ > LC_ALL=C sort -u > all-gitfiles; > \ > LC_ALL=C comm -1 -3 all-distfiles all-gitfiles > > missing-distfiles; \ > if test -s missing-distfiles; then > \ > diff --git a/boot.sh b/boot.sh > index 05dd359..d9a48fa 100755 > --- a/boot.sh > +++ b/boot.sh > @@ -1,2 +1,3 @@ > #! /bin/sh > +[ -d .git ] && git submodule update --init --recursive > autoreconf --install --force > diff --git a/python b/python > new file mode 160000 > index 0000000..dd34813 > --- /dev/null > +++ b/python > @@ -0,0 +1 @@ > +Subproject commit dd348134a6e186725fe5aaa8d0a482fee0a88f2a > > ... < snip deleting python/ > > > Feel free to check it out and verify that it works. I split out the > python directory here as a test > https://github.com/otherwiseguy/python-ovs. > > As far as versioning, we could just pick a starting value of the new > library of something like 3.0.0, breaking it out from the OVS > versioning. > > Terry > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Tue, Nov 15, 2016 at 11:29:51AM -0600, Terry Wilson wrote: > The Python library isn't dependent on the code in the OVS tree. It > being in-tree has a few shortcomings. My rationale for recommending > the split: > > * Simple features and bugfixes for the Python lib can't be used by > other projects (like Neutron) until the very latest OVS release is > widely supported I am not sure that I understand why. Why would Neutron and other projects be more willing to use a pre-release version of an OVS Python library, than a pre-release version of OVS itself? Or do you mean that the OVS Python library would release more frequently than OVS itself? The following two bullets are related: > * Python 3 support is only available in version 2.6.0+ even though the > code would work for previous releases > > * Implying that the Python lib and OVS versions are related when they > aren't is confusing The Python library and OVS version numbers are indeed related, in that a given version of OVS is meant to work with the same version of the Python library, and vice versa. Maybe there is some flexibility in working with different versions, but if so then this is more by accident than design. Scanning the recent commits to the repository, it appears that the main dependency is just that the main OVS code requires a new-enough version of the Python code. Probably, this is not a big deal, but the split-off repos should explain the situation somewhere. As you suggest, maybe the Python code from 2.6.x would work with earlier versions of OVS. But this was not a foreseen use, so I would not want to recommend that users do this. If we split the repos and start thinking about this kind of possibility, then I agree that similar combinations could make sense for OVS 2.7.x+. > * A separate repo would allow adding committers that are familiar with > the Python code, but less familiar with the C code. I'd rather have only "committers", not "OVS committers" and "OVS Python library committers". We don't have "OVS committers" and "OVS Linux datapath committers" and "OVS Windows datapath committers" and "OVS-DPDK committers", for example. We trust our committers not to commit code without obtaining assurance that the code makes sense, through careful coding and at least one quality review. Part of that trust is trusting committers to recognize when their own understanding of a piece code is limited and therefore that they should obtain more or better reviews. In other words, I want to trust programmers who do not know C to commit to the C parts of OVS anyway, because they will take the trouble to get good advice. > * As a Python-only project, it could be more easily tested, built, and > packaged according to Python project best practices. I think that most of the Python tests run some part of OVS. I'm not sure how effective the Python library tests can be if OVS isn't available. > The current build process requires the Python code, but this is easily > remedied by using git submodules. People tend to reflexively dislike > them, but they are perfect for this application. The OVS tree can lock > to any particular commit for the new python-ovs repo and be guaranteed > that changes don't break the ovs build. I made a fork to show how > simple the change would be: > > https://github.com/otherwiseguy/ovs/commit/6766131b42807829ea78dbc43d164db8926030e7 > > commit 6766131b42807829ea78dbc43d164db8926030e7 > Author: Terry Wilson <twilson@redhat.com> > Date: Mon Nov 14 16:03:43 2016 -0600 > > Move python dir to a submodule It's a straightforward change, yes, if it's valuable. Documentation and webpage updates would be needed to explain the new build process.
I'm worried that my response here effectively shut down the proposal. That wasn't my goal; if it's valuable, let's do it, but I want more information first. On Mon, Nov 21, 2016 at 09:42:46PM -0800, Ben Pfaff wrote: > On Tue, Nov 15, 2016 at 11:29:51AM -0600, Terry Wilson wrote: > > The Python library isn't dependent on the code in the OVS tree. It > > being in-tree has a few shortcomings. My rationale for recommending > > the split: > > > > * Simple features and bugfixes for the Python lib can't be used by > > other projects (like Neutron) until the very latest OVS release is > > widely supported > > I am not sure that I understand why. Why would Neutron and other > projects be more willing to use a pre-release version of an OVS Python > library, than a pre-release version of OVS itself? Or do you mean that > the OVS Python library would release more frequently than OVS itself? > > The following two bullets are related: > > > * Python 3 support is only available in version 2.6.0+ even though the > > code would work for previous releases > > > > * Implying that the Python lib and OVS versions are related when they > > aren't is confusing > > The Python library and OVS version numbers are indeed related, in that a > given version of OVS is meant to work with the same version of the > Python library, and vice versa. Maybe there is some flexibility in > working with different versions, but if so then this is more by accident > than design. > > Scanning the recent commits to the repository, it appears that the main > dependency is just that the main OVS code requires a new-enough version > of the Python code. Probably, this is not a big deal, but the split-off > repos should explain the situation somewhere. > > As you suggest, maybe the Python code from 2.6.x would work with earlier > versions of OVS. But this was not a foreseen use, so I would not want > to recommend that users do this. If we split the repos and start > thinking about this kind of possibility, then I agree that similar > combinations could make sense for OVS 2.7.x+. > > > * A separate repo would allow adding committers that are familiar with > > the Python code, but less familiar with the C code. > > I'd rather have only "committers", not "OVS committers" and "OVS Python > library committers". We don't have "OVS committers" and "OVS Linux > datapath committers" and "OVS Windows datapath committers" and "OVS-DPDK > committers", for example. We trust our committers not to commit code > without obtaining assurance that the code makes sense, through careful > coding and at least one quality review. Part of that trust is trusting > committers to recognize when their own understanding of a piece code is > limited and therefore that they should obtain more or better reviews. > In other words, I want to trust programmers who do not know C to commit > to the C parts of OVS anyway, because they will take the trouble to get > good advice. > > > * As a Python-only project, it could be more easily tested, built, and > > packaged according to Python project best practices. > > I think that most of the Python tests run some part of OVS. I'm not > sure how effective the Python library tests can be if OVS isn't > available. > > > The current build process requires the Python code, but this is easily > > remedied by using git submodules. People tend to reflexively dislike > > them, but they are perfect for this application. The OVS tree can lock > > to any particular commit for the new python-ovs repo and be guaranteed > > that changes don't break the ovs build. I made a fork to show how > > simple the change would be: > > > > https://github.com/otherwiseguy/ovs/commit/6766131b42807829ea78dbc43d164db8926030e7 > > > > commit 6766131b42807829ea78dbc43d164db8926030e7 > > Author: Terry Wilson <twilson@redhat.com> > > Date: Mon Nov 14 16:03:43 2016 -0600 > > > > Move python dir to a submodule > > It's a straightforward change, yes, if it's valuable. > > Documentation and webpage updates would be needed to explain the new > build process.
My take was that the primary benefit that would justify pursuing this would be an independent release schedule, but we don't have any changes (committed or pending) that raise that issue. The prime example for me was Python 3 support, where we really wanted it released, but had to wait about 8 months for the next ovs release. The new release schedule means more like 5-6 months, max, but perhaps a similar situation could arise. My suggestion is to wait until something like that comes up again. The rest isn't a strong enough justification to me.
diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..db21481 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "python"] + path = python + url = https://github.com/otherwiseguy/python-ovs diff --git a/Makefile.am b/Makefile.am index a14d48b..2ce40ee 100644 --- a/Makefile.am +++ b/Makefile.am @@ -225,7 +225,7 @@ dist-hook-git: distfiles (cd datapath && $(MAKE) distfiles); \ (cat distfiles; sed 's|^|datapath/|' datapath/distfiles) | \ LC_ALL=C sort -u > all-distfiles; \ - (cd $(srcdir) && git ls-files) | grep -v '\.gitignore$$' | \ + (cd $(srcdir) && git ls-files) | grep -v 'python\|\.git\(ignore\|modules\)$$' | \ LC_ALL=C sort -u > all-gitfiles; \ LC_ALL=C comm -1 -3 all-distfiles all-gitfiles > missing-distfiles; \ if test -s missing-distfiles; then \ diff --git a/boot.sh b/boot.sh index 05dd359..d9a48fa 100755 --- a/boot.sh +++ b/boot.sh @@ -1,2 +1,3 @@ #! /bin/sh +[ -d .git ] && git submodule update --init --recursive autoreconf --install --force diff --git a/python b/python new file mode 160000 index 0000000..dd34813 --- /dev/null +++ b/python @@ -0,0 +1 @@ +Subproject commit dd348134a6e186725fe5aaa8d0a482fee0a88f2a
The Python library isn't dependent on the code in the OVS tree. It being in-tree has a few shortcomings. My rationale for recommending the split: * Simple features and bugfixes for the Python lib can't be used by other projects (like Neutron) until the very latest OVS release is widely supported * Python 3 support is only available in version 2.6.0+ even though the code would work for previous releases * Implying that the Python lib and OVS versions are related when they aren't is confusing * A separate repo would allow adding committers that are familiar with the Python code, but less familiar with the C code. * As a Python-only project, it could be more easily tested, built, and packaged according to Python project best practices. The current build process requires the Python code, but this is easily remedied by using git submodules. People tend to reflexively dislike them, but they are perfect for this application. The OVS tree can lock to any particular commit for the new python-ovs repo and be guaranteed that changes don't break the ovs build. I made a fork to show how simple the change would be: https://github.com/otherwiseguy/ovs/commit/6766131b42807829ea78dbc43d164db8926030e7 commit 6766131b42807829ea78dbc43d164db8926030e7 Author: Terry Wilson <twilson@redhat.com> Date: Mon Nov 14 16:03:43 2016 -0600 Move python dir to a submodule ... < snip deleting python/ > Feel free to check it out and verify that it works. I split out the python directory here as a test https://github.com/otherwiseguy/python-ovs. As far as versioning, we could just pick a starting value of the new library of something like 3.0.0, breaking it out from the OVS versioning. Terry