Discussion:
[PR PATCH] More py2 py3 compat
Github bot account via samba-technical
2018-05-06 10:05:39 UTC
Permalink
There is a new pull request by noelpower against master on the Samba Samba Github repository

https://github.com/noelpower/samba more_py2_py3_compat
https://github.com/samba-team/samba/pull/176

More py2 py3 compat
Some general py2/py3 compatibility fixes.
* b64encode results in bytes in python3 but we invariably treat the result in our code as string. This changes converts all usage of b64encode to decode the result, this will result in unicode in python2. Treating text as a text type (e.g. in python2) should ensure some sanity down the line (and should expose anywhere we don't really treat the result as a string)
* keys() now returns 'dict' class not a list, enclose result of .keys with list() function where it seems that makes sense
* 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)
* md5 takes bytes in python3, some patches to make sure this is the case
* StringIO has moved to 'io' in python3, use the compat wrapper
* some missed exception compatibility fixes
* .next() method not longer exists in python3

A patch file from https://github.com/samba-team/samba/pull/176.patch is attached
Github bot account via samba-technical
2018-05-06 10:09:01 UTC
Permalink
There is an updated pull request by noelpower against master on the Samba Samba Github repository

https://github.com/noelpower/samba more_py2_py3_compat
https://github.com/samba-team/samba/pull/176

More py2 py3 compat
Some general py2/py3 compatibility fixes.
* b64encode results in bytes in python3 but we invariably treat the result in our code as string. This changes converts all usage of b64encode to decode the result, this will result in unicode in python2. Treating text as a text type (e.g. in python2) should ensure some sanity down the line (and should expose anywhere we don't really treat the result as a string)
* keys() now returns 'dict' class not a list, enclose result of .keys with list() function where it seems that makes sense
* 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)
* md5 takes bytes in python3, some patches to make sure this is the case
* StringIO has moved to 'io' in python3, use the compat wrapper
* some missed exception compatibility fixes
* .next() method not longer exists in python3

A patch file from https://github.com/samba-team/samba/pull/176.patch is attached
Douglas Bagnall via samba-technical
2018-05-06 11:51:40 UTC
Permalink
Post by Github bot account via samba-technical
* 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)
From 6d1a2b54eab7897b1ff71dc84ff6c16df2d29c3e Mon Sep 17 00:00:00 2001
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
---
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...
Post by Github bot account via samba-technical
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
self.outf.write("Namespaces[%d]%s:\n" % (
len(fti.entries), tln_string))
e = fti.entries[i]
This would be better as

- for i in xrange(0, len(fti.entries)):
- e = fti.entries[i]
Post by Github bot account via samba-technical
flags = e.flags
@@ -3403,7 +3403,7 @@ def run(self, domain=None, sambaopts=None, localdcopts=None, versionopts=None,
idx = None
v = update_upn_vals[i]
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

Loading...