[PR PATCH] More py2 py3 compat
Douglas Bagnall
douglas.bagnall at catalyst.net.nz
Sun May 6 11:51:40 UTC 2018
On 06/05/18 22:05, Github bot (Noel Power) wrote:
> * xrange -> range Note: there is one instance of this where it might
> make a difference (see commit message) due to the potential amount
> of items in the range. Advice needed here, might need to drop one
> of these patches at the moment. Six apparently has a wrapper we
> could use instead (but CI doesn't support yet... I think)
Which I assume is referring to this one:
> From 6d1a2b54eab7897b1ff71dc84ff6c16df2d29c3e Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 4 May 2018 12:20:36 +0100
> Subject: [PATCH 10/17] samba_tool: replace xrange -> range
>
> This one might be a better candidate for using six.moves as there
> are perhaps issues with the amount of elements involved
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
> python/samba/netcmd/domain.py | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
I wouldn't worry too much because samba-tool is slow anyway, but...
> diff --git a/python/samba/netcmd/domain.py b/python/samba/netcmd/domain.py
> index 1e242dea62e..5c92bb7f393 100644
> --- a/python/samba/netcmd/domain.py
> +++ b/python/samba/netcmd/domain.py
> @@ -2011,7 +2011,7 @@ def write_forest_trust_info(self, fti, tln=None, collisions=None):
> self.outf.write("Namespaces[%d]%s:\n" % (
> len(fti.entries), tln_string))
>
> - for i in xrange(0, len(fti.entries)):
> + for i in range(0, len(fti.entries)):
> e = fti.entries[i]
This would be better as
- for i in xrange(0, len(fti.entries)):
- e = fti.entries[i]
+ for i, e in enumerate(fti.entries):
or you could go further. To take the next example:
> flags = e.flags
> @@ -3403,7 +3403,7 @@ def run(self, domain=None, sambaopts=None, localdcopts=None, versionopts=None,
>
> for upn in add_upn:
> idx = None
> - for i in xrange(0, len(update_upn_vals)):
> + for i in range(0, len(update_upn_vals)):
> v = update_upn_vals[i]
> if v.lower() != upn.lower():
> continue
@@ -3401,15 +3400,11 @@ class cmd_domain_trust_namespaces(DomainTrustCommand):
update_spn_vals.extend(stored_spn_vals)
for upn in add_upn:
- idx = None
- for i in xrange(0, len(update_upn_vals)):
- v = update_upn_vals[i]
- if v.lower() != upn.lower():
- continue
- idx = i
- break
- if idx is not None:
- raise CommandError("Entry already present for value[%s] specified for --add-upn-suffix" % upn)
+ for v in update_upn_vals:
+ if v.lower() == upn.lower():
+ raise CommandError("Entry already present for "
+ "value[%s] specified for "
+ "--add-upn-suffix" % upn)
update_upn_vals.append(upn)
replace_upn = True
If efficiency really mattered, it would be better to replace the
update_upn_vals with something like an ordereddict where the keys were
the lowercase upns. Then it would become:
+ if upn.lower() in update_upn_vals_lower:
+ raise CommandError(...)
+ update_upn_vals_lower[upn.lower()] = upn
replace_upn = True
but going to enumerate() is already an improvement in 2 and 3 and
should be simple enough.
cheers,
Douglas
More information about the samba-technical
mailing list