ctdb/utils/etcd/ctdb_etcd_lock python => python3

[L.P.H. van Belle]

Hai Martin, 

 

> -----Oorspronkelijk bericht-----
> Van: Martin Schwenke [mailto:martin at meltin.net] 
> Verzonden: woensdag 28 april 2021 12:50
> Aan: L.P.H. van Belle
> CC: samba-technical at lists.samba.org 
> Onderwerp: Re: ctdb/utils/etcd/ctdb_etcd_lock python => python3
> 
> Hi Louis,
> 
> On Fri, 23 Apr 2021 14:32:12 +0200, L.P.H. van Belle <belle at bazuin.nl>
> wrote:
> 
> > > How exactly does the build fail?  
> 
> > Well, i use cowbuild/pbuilder in Debian 10 and in the end lintian
> > checks the packages and errors with.
> > 
> > E: ctdb: python-script-but-no-python-dep 
> usr/lib/x86_64-linux-gnu/ctdb/ctdb_etcd_lock #!python3 
> > Even after i added in debian/control
> > ${python3:Depends} for CTDB package
> 
> OK, Debian build fails.  Now I understand...  :-)
:-) yeah, i only do Debian and Ubuntu.. 

> 
> > > > I dont know if its really needed but all python 
> references are used
> > > > with /usr/bin/env python3. 
> > > > 
> > > > Just to besure im not breaking something in ctdb here. 
> > > > Can someone confirm this? 
> > > > 
> > > > My fix.
> > > > 
> > > > --- a/ctdb/utils/etcd/ctdb_etcd_lock
> > > > +++ b/ctdb/utils/etcd/ctdb_etcd_lock
> > > > @@ -1,4 +1,4 @@
> > > > -#!/usr/bin/python
> > > > +#!/usr/bin/env python3  
> > > 
> > > I'd like to be able to give a better answer but I'm not 
> entirely sure
> > > that the script works with python3 without any other changes. 
> > >  It looks
> > > like it should but I don't have a trivial way of testing it 
> > > (or time to setup a test environment).
> > > 
> > > Given that /usr/bin/python isn't supported anymore, we 
> might as well
> > > change it as above see what bug reports get generated.
> > > 
> > > If you're keen to improve the script then you could try 
> to clean it up
> > > and get rid of warnings from pylint and flake8.  getting 
> rid of the
> > > obvious whitespace issues might make any python3 issues 
> more obvious.
> > > Perhaps yapf3 would be useful?  
> > 
> > I really wished i had these skills todo that, i would then.. 
> > :-( 
> 
> Let's just make the obvious change that you suggested.   If we only
> support Python 3 then we can't leave this script at Python 2.  If it
> breaks then someone can report a bug and hopefully send a patch.
> 
> Can you post a git version of the patch with your 
> Signed-off-by: line or
> should I do that and just push it with my Reviewed-by:?

Yes please do, im atm not setup for git and before i push things into git
I need to be better with my git skills, its to easy for me to mess things
up in the samba git.  :-/ 

> 
> Does the fix need to be backported to released versions or is master
> enough?

I think master is ok. 

> 
> We don't have any tests for that script and I don't have time to write
> one - I'm guessing that to limit the dependencies we'd need to fake up
> a simple etcd or something... seems like a lot of work.
Well, the current fix in the end for the debian packaging was the 
 ${python3:Depends} in the ctdb package so that ok now. 

We could add python2 also but didn't we move to python3 because python2 is predicated? 

> 
> I've done some hacking using autopep8 (rather than yapf3) and then
> fixing a bunch of things reported by flake8.  I'll look at 
> pushing some of them some time later...
> 
> Thanks...

Well, Thank you Martin for the replies. 
Have a great day. 


Greetz, 

Louis