[ovs-dev] Proposal to move the Python lib to its own repo
diff mbox

Message ID CAA4u48E8791aFihd-uYMMKn9_4pQX2wgHs6fy-3-8gNwP8AXSw@mail.gmail.com
State Deferred
Headers show

Commit Message

Terry Wilson Nov. 15, 2016, 5:29 p.m. UTC
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

Comments

Russell Bryant Nov. 21, 2016, 8:34 p.m. UTC | #1
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
>
Ben Pfaff Nov. 22, 2016, 5:42 a.m. UTC | #2
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.
Ben Pfaff Dec. 12, 2016, 9:34 p.m. UTC | #3
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.
Russell Bryant Dec. 16, 2016, 4:07 p.m. UTC | #4
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.

Patch
diff mbox

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