Re: [ovs-dev] [RFC] [PATCH] 1/1 Add multi-column index support for the Python IDL

2018-04-12 Thread Flavio Leitner
On Wed, Apr 11, 2018 at 03:54:24PM -0500, Terry Wilson wrote:
> On Wed, Apr 11, 2018 at 12:52 PM, Flavio Leitner  wrote:
> > On Wed, Apr 11, 2018 at 07:23:07PM +0200, Timothy Redaelli wrote:
> >> On Tue, 10 Apr 2018 15:20:54 -0700
> >> Ben Pfaff  wrote:
> >>
> >> > On Mon, Apr 09, 2018 at 03:03:03PM -0500, twil...@redhat.com wrote:
> >> > > From: Terry Wilson 
> >> > >
> >> > > This adds multi-column index support for the Python IDL that is
> >> > > similar to the feature in the C IDL.
> >> > >
> >> > > Signed-off-by: Terry Wilson 
> >> >
> >> > Thanks for working on this.
> >> >
> >> > I think that this adds a new Python module dependency on
> >> > "sortedcontainers".  I didn't have it installed for python2 or
> >> > python3--at least on Debian, it is not installed by default--so many
> >> > tests failed.  I guess that we should at least document that, and
> >> > probably it means that there should be new dependencies.
> >> >
> >> > (When I installed the Python module, everything was fine.)
> >>
> >> Hi,
> >> unfortunately "python-sortedcontainers" packages are not available on
> >> RHEL repositories.
> >>
> >> This means, if this commit is approved, it will not be possible to build
> >> OVS on RHEL anymore and, if we skip the tests, it will not be possible
> >> to use the python OVS extensions.
> >>
> >> so I suggest, if possible, to find and alternative to
> >> "sortedcontainers".
> >
> > Hi Terry,
> >
> > Not trying to be the obstructionist here, but as you pointed out in
> > the cover letter and Tim highlighted here, that dependency isn't
> > packaged in neither RHEL (7 or 8) or EPEL, which means it breaks OVS
> > backwards compatibility. I suspect others not-so-recent distros might
> > have the same problem.  For instance, only F27 includes that IIRC.
> >
> > Do you have a plan to address that issue?
> 
> My plan, if people are in general OK with adding the dependency at all
> (and the architecture of the RFC patch), is 1) Take the Fedora
> packages and try to get them in RHEL and 2) make only the feature
> dependent on sortedcontainers with some simple try/except magic.
> Another option would be to just take the part of sortedcontainers
> (which is Apache 2.0) that we need, drop it in the tree, and have the
> exception on import actually import from the local copy. In general I
> don't like copying projects into other projects, though.
> sortedcontainers is a very mature, fast, pure-python library. It would
> be handy to be able to use it even in other parts of the library. For
> instance, we call sorted() a lot in data.py for doing things like
> to_json(), to_python(), to_string(), etc. Sorting on each call instead
> of storing in a sorted object in the first place. And those methods
> are called *a lot*.

The problem on getting the it packaged on those distributions is that
they won't update older versions.  It is today in F27, but it will be
very hard to make it available on F26, for instance.

I am not a big fan of copying either, but it will depend on our options.
If you use "try/except", for example, then you don't get the improvements
until that package is available. Does that work for you?

-- 
Flavio

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC] [PATCH] 1/1 Add multi-column index support for the Python IDL

2018-04-11 Thread Terry Wilson
On Wed, Apr 11, 2018 at 12:52 PM, Flavio Leitner  wrote:
> On Wed, Apr 11, 2018 at 07:23:07PM +0200, Timothy Redaelli wrote:
>> On Tue, 10 Apr 2018 15:20:54 -0700
>> Ben Pfaff  wrote:
>>
>> > On Mon, Apr 09, 2018 at 03:03:03PM -0500, twil...@redhat.com wrote:
>> > > From: Terry Wilson 
>> > >
>> > > This adds multi-column index support for the Python IDL that is
>> > > similar to the feature in the C IDL.
>> > >
>> > > Signed-off-by: Terry Wilson 
>> >
>> > Thanks for working on this.
>> >
>> > I think that this adds a new Python module dependency on
>> > "sortedcontainers".  I didn't have it installed for python2 or
>> > python3--at least on Debian, it is not installed by default--so many
>> > tests failed.  I guess that we should at least document that, and
>> > probably it means that there should be new dependencies.
>> >
>> > (When I installed the Python module, everything was fine.)
>>
>> Hi,
>> unfortunately "python-sortedcontainers" packages are not available on
>> RHEL repositories.
>>
>> This means, if this commit is approved, it will not be possible to build
>> OVS on RHEL anymore and, if we skip the tests, it will not be possible
>> to use the python OVS extensions.
>>
>> so I suggest, if possible, to find and alternative to
>> "sortedcontainers".
>
> Hi Terry,
>
> Not trying to be the obstructionist here, but as you pointed out in
> the cover letter and Tim highlighted here, that dependency isn't
> packaged in neither RHEL (7 or 8) or EPEL, which means it breaks OVS
> backwards compatibility. I suspect others not-so-recent distros might
> have the same problem.  For instance, only F27 includes that IIRC.
>
> Do you have a plan to address that issue?

My plan, if people are in general OK with adding the dependency at all
(and the architecture of the RFC patch), is 1) Take the Fedora
packages and try to get them in RHEL and 2) make only the feature
dependent on sortedcontainers with some simple try/except magic.
Another option would be to just take the part of sortedcontainers
(which is Apache 2.0) that we need, drop it in the tree, and have the
exception on import actually import from the local copy. In general I
don't like copying projects into other projects, though.
sortedcontainers is a very mature, fast, pure-python library. It would
be handy to be able to use it even in other parts of the library. For
instance, we call sorted() a lot in data.py for doing things like
to_json(), to_python(), to_string(), etc. Sorting on each call instead
of storing in a sorted object in the first place. And those methods
are called *a lot*.

Terry
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC] [PATCH] 1/1 Add multi-column index support for the Python IDL

2018-04-11 Thread Ben Pfaff
On Wed, Apr 11, 2018 at 02:52:51PM -0300, Flavio Leitner wrote:
> On Wed, Apr 11, 2018 at 07:23:07PM +0200, Timothy Redaelli wrote:
> > On Tue, 10 Apr 2018 15:20:54 -0700
> > Ben Pfaff  wrote:
> > 
> > > On Mon, Apr 09, 2018 at 03:03:03PM -0500, twil...@redhat.com wrote:
> > > > From: Terry Wilson 
> > > > 
> > > > This adds multi-column index support for the Python IDL that is
> > > > similar to the feature in the C IDL.
> > > > 
> > > > Signed-off-by: Terry Wilson   
> > > 
> > > Thanks for working on this.
> > > 
> > > I think that this adds a new Python module dependency on
> > > "sortedcontainers".  I didn't have it installed for python2 or
> > > python3--at least on Debian, it is not installed by default--so many
> > > tests failed.  I guess that we should at least document that, and
> > > probably it means that there should be new dependencies.
> > > 
> > > (When I installed the Python module, everything was fine.)
> > 
> > Hi,
> > unfortunately "python-sortedcontainers" packages are not available on
> > RHEL repositories.
> > 
> > This means, if this commit is approved, it will not be possible to build
> > OVS on RHEL anymore and, if we skip the tests, it will not be possible
> > to use the python OVS extensions.
> > 
> > so I suggest, if possible, to find and alternative to
> > "sortedcontainers".
> 
> Hi Terry,
> 
> Not trying to be the obstructionist here, but as you pointed out in
> the cover letter and Tim highlighted here, that dependency isn't
> packaged in neither RHEL (7 or 8) or EPEL, which means it breaks OVS
> backwards compatibility. I suspect others not-so-recent distros might
> have the same problem.  For instance, only F27 includes that IIRC.
> 
> Do you have a plan to address that issue?

We used to install some Python compatibility modules on some OSes.  See
commit e23812fc60af (Increase prerequisite from Python 2.4 to Python
2.7.) to see how we did it before (which was pretty simple).  The same
thing might work here.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC] [PATCH] 1/1 Add multi-column index support for the Python IDL

2018-04-11 Thread Flavio Leitner
On Wed, Apr 11, 2018 at 07:23:07PM +0200, Timothy Redaelli wrote:
> On Tue, 10 Apr 2018 15:20:54 -0700
> Ben Pfaff  wrote:
> 
> > On Mon, Apr 09, 2018 at 03:03:03PM -0500, twil...@redhat.com wrote:
> > > From: Terry Wilson 
> > > 
> > > This adds multi-column index support for the Python IDL that is
> > > similar to the feature in the C IDL.
> > > 
> > > Signed-off-by: Terry Wilson   
> > 
> > Thanks for working on this.
> > 
> > I think that this adds a new Python module dependency on
> > "sortedcontainers".  I didn't have it installed for python2 or
> > python3--at least on Debian, it is not installed by default--so many
> > tests failed.  I guess that we should at least document that, and
> > probably it means that there should be new dependencies.
> > 
> > (When I installed the Python module, everything was fine.)
> 
> Hi,
> unfortunately "python-sortedcontainers" packages are not available on
> RHEL repositories.
> 
> This means, if this commit is approved, it will not be possible to build
> OVS on RHEL anymore and, if we skip the tests, it will not be possible
> to use the python OVS extensions.
> 
> so I suggest, if possible, to find and alternative to
> "sortedcontainers".

Hi Terry,

Not trying to be the obstructionist here, but as you pointed out in
the cover letter and Tim highlighted here, that dependency isn't
packaged in neither RHEL (7 or 8) or EPEL, which means it breaks OVS
backwards compatibility. I suspect others not-so-recent distros might
have the same problem.  For instance, only F27 includes that IIRC.

Do you have a plan to address that issue?

-- 
Flavio

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC] [PATCH] 1/1 Add multi-column index support for the Python IDL

2018-04-11 Thread Timothy Redaelli
On Tue, 10 Apr 2018 15:20:54 -0700
Ben Pfaff  wrote:

> On Mon, Apr 09, 2018 at 03:03:03PM -0500, twil...@redhat.com wrote:
> > From: Terry Wilson 
> > 
> > This adds multi-column index support for the Python IDL that is
> > similar to the feature in the C IDL.
> > 
> > Signed-off-by: Terry Wilson   
> 
> Thanks for working on this.
> 
> I think that this adds a new Python module dependency on
> "sortedcontainers".  I didn't have it installed for python2 or
> python3--at least on Debian, it is not installed by default--so many
> tests failed.  I guess that we should at least document that, and
> probably it means that there should be new dependencies.
> 
> (When I installed the Python module, everything was fine.)

Hi,
unfortunately "python-sortedcontainers" packages are not available on
RHEL repositories.

This means, if this commit is approved, it will not be possible to build
OVS on RHEL anymore and, if we skip the tests, it will not be possible
to use the python OVS extensions.

so I suggest, if possible, to find and alternative to
"sortedcontainers".

Thank you

> Thanks,
> 
> Ben.
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev



-- 
Timothy Redaelli
Software Engineer
Red Hat Italia
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC] [PATCH] 1/1 Add multi-column index support for the Python IDL

2018-04-10 Thread Ben Pfaff
On Mon, Apr 09, 2018 at 03:03:03PM -0500, twil...@redhat.com wrote:
> From: Terry Wilson 
> 
> This adds multi-column index support for the Python IDL that is
> similar to the feature in the C IDL.
> 
> Signed-off-by: Terry Wilson 

Thanks for working on this.

I think that this adds a new Python module dependency on
"sortedcontainers".  I didn't have it installed for python2 or
python3--at least on Debian, it is not installed by default--so many
tests failed.  I guess that we should at least document that, and
probably it means that there should be new dependencies.

(When I installed the Python module, everything was fine.)

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev