[PATCH] even more python patches

[Noel Power]

Hi Douglas

On 28/10/2018 21:56, Douglas Bagnall via samba-technical wrote:
> I know you don't like to miss out on reviewing trivial python patches,
> so I prepared another batch.
>
> *Most* of these are easy.
>
> They passed CI with the others, and are running on their own at
> https://gitlab.com/samba-team/devel/samba/pipelines/34610880
>
> cheers,
> Douglas

> [PATCH 01/17] python dbcheck:  don't use mutable default args

I think already this is handled, 'attrs' is not modified it is passed to 
check_object which makes a copy of it  (there is also an accompanying 
comment about the pass-by-reference 'attrs' variable). I am not against 
the change per-se, maybe it will prevent accidental modification of 
attrs if 'check_database' is changed in the future.

OTOH I don't like the fact the default is expressed as a tuple, to me it 
sortof documents that the expected param type (which is list) now looks 
like it needs to be a tuple.

> [PATCH 05/17] selftesthelpers: use immutable value for extra_path
>
> -def planpythontestsuite(env, module, name=None, extra_path=[], 
> py3_compatible=False):
> +def planpythontestsuite(env, module, name=None, extra_path=(), 
> py3_compatible=False):
>      if name is None:
>          name = module
>      pypath = list(extra_path)
>
like above  extra_path is never used directly & pypath is copied

I wont object to the 2 above if you wish to make those changes, I just 
think we are fixing something that is already catered for.

> [PATCH 08/17] tests/py/rodc_rwdc: avoid py2/py3 .next compat issues
>
> class RodcRwdcTests(password_lockout_base.BasePasswordTestCase):
> -    counter = itertools.count(1).next
> +    next_user_number = [1]
>

I didn't get why a list was used for next_user_number, was there some 
reason why just a normal variable wouldn't work? Anyway I think if the 
orig style of code was to be ported below would be more in line and 
*should* (hasn't been tested) work

@@ -115,7 +115,6 @@ def get_server_ref_from_samdb(samdb):


  class RodcRwdcCachedTests(password_lockout_base.BasePasswordTestCase):
-    counter = itertools.count(1).next

      def _check_account_initial(self, dn):
          self.force_replication()
@@ -686,7 +685,7 @@ class 
RodcRwdcCachedTests(password_lockout_base.BasePasswordTestCase):


  class RodcRwdcTests(password_lockout_base.BasePasswordTestCase):
-    counter = itertools.count(1).next
+    counter = itertools.count(1,1)

      def force_replication(self, base=None):
          if base is None:
@@ -982,7 +981,7 @@ class 
RodcRwdcTests(password_lockout_base.BasePasswordTestCase):
          self._test_add_modify_delete()

      def _new_user(self):
-        username = "u%sX%s" % (self.tag[:12], self.counter())
+        username = "u%sX%s" % (self.tag[:12], next(self.counter()))
          password = 'password#1'
          dn = 'CN=%s,CN=Users,%s' % (username, self.base_dn)
          o = {


> [PATCH 12/17] s4/scripting/*:  py3 compatible print
>

> diff --git a/source4/scripting/bin/findprovisionusnranges 
> b/source4/scripting/bin/findprovisionusnranges
> index ee9da3d421e..65e5b3838bc 100755
> --- a/source4/scripting/bin/findprovisionusnranges
> +++ b/source4/scripting/bin/findprovisionusnranges
> @@ -18,7 +18,7 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  #
> -
> +from __future__ import print_function
>  import sys
>  import optparse
>  sys.path.insert(0, "bin/python")
do we need the 'from __future__ import print_function here?

same in patch for source4/scripting/bin/mymachinepw and 
python/samba/common.py

don't see we need it for these, if this is the case let's avoid another 
unnecessary import


Noel