[SSSD] [PATCHES] SSSDConfig: Port missing parts to python3
Lukas Slebodnik
lslebodn at redhat.com
Thu Jan 29 15:39:01 UTC 2015
On (29/01/15 13:09), Petr Viktorin wrote:
>On 01/29/2015 01:05 PM, Bohuslav Kabrda wrote:
>>----- Original Message -----
>>>ehlo,
>>>
>>>some parts of sssd was not properly ported to python3.
>>>I know there were changes related to unicode, string and bytes.
>>>I am not sure whether my patches for read and open are correct,
>>>especially patch "SSSDConfig: os.write".
>>>
>>>SSSDConfig (python-sssdconfig is used by authconfig and ipa-client-install?
>>>So I don't want to break it.
>>>
>>>Please review patches or propose better version.
>>>
>>>LS
>>
>>Not sure why I missed so many things in my original patch (maybe I searched
>> just for *.py, not *.py.in ;)). Anyway:
and IIRC sbus_codegen was not in git when you sent patches.
>>- Patch 1 LGTM, I only don't understand why you need to import "exceptions"
>>module (and why only in Python 2). Note that Python 2 documentation says
>>that "This module never needs to be imported explicitly". [1]
It might have been used due to older version of python (RHEL5),
which we do not support any more.
I removed it.
>>- Patch 2 LGMT, but I think you could just write:
>>
>>'long': long if sys.version_info[0] == 2 else int
>>
That's nicer.
>>(but that's just nitpicking...)
>>- Patch 3: this depends. Is the object that you're writing binary or text?
>>I think what you want to do is keep "wb" mode and encode output to a certain
>>encoding (utf8?). So this could be:
>>
>>of = open(outputfile, "wb")
>>output = self.dump(self.opts).encode("utf-8")
>>of.write(output)
Output file should be valid sssd configuration file which if text file.
I used your proposal.
>>The problem is that the "str" type in Python 2 is sort of polymorphic, in
>>that it can behave both as binary bytestring and as unicode - but .encode()
>>works on it nonetheless. In Python 3 you have either "bytes" (binary string)
>>or "str" (unicode text string) and you have to choose which one you want
>>to write ("wb" mode is for "bytes", while "w" is for "str").
>>If "self.dump(self.opts)" returns "str" in Python 3, then I suggest you use
>>'.encode("utf-8")' on it and leave the mode "wb".
>>This should keep working in Python 2.
>>- Patch 4 LGTM
>>- Patch 5 LGTM
>>
>>Slavek
>>
>>[1] https://docs.python.org/2/library/exceptions.html#module-exceptions
>>
>
>Patch 1/5: Note that even under Python 2, `except ValueError, KeyError:`
>would not work (it catches ValueError, and stores the exception instance
>under the name KeyError); please test these changes well.
>
It means that in python2 KeyError was not handled at all.
python2 documentation says:
"If an exception occurs which does not match the exception named in the except
clause, it is passed on to outer try statements; if no handler is found, it is
an unhandled exception and execution stops with a message as shown above."
In my patch, you can see that different exception was thrown (ParsingError) in
handler. I'm not sure hot to test it therefore you are in CC ;-)
Shall I remove KeyError?
>Patch 4/5: you can use io.StringIO in both 2.6+ and 3.x+
>
Yes, but...
Traceback (most recent call last):
File "../src/sbus/sbus_codegen", line 931, in <module>
main()
File "../src/sbus/sbus_codegen", line 919, in main
generate_source(parser.parsed_interfaces, filename, options.include)
File "../src/sbus/sbus_codegen", line 600, in generate_source
out("/* The following definitions are auto-generated from %s */", basename)
File "../src/sbus/sbus_codegen", line 217, in out
sys.stdout.write(str)
TypeError: unicode argument expected, got 'str'
So I'm find with such solution for StringIO. The sbus_codegen is python script
which generate C source file. I do not expect any unicode characters.
But I'm not opposed better version.
Thank you very much for review.
I squashed some small patches.
LS
-------------- next part --------------
>From ec7d5ddae385dcc35c80dc0b77d7c31a4bb710c7 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Thu, 29 Jan 2015 09:46:27 +0100
Subject: [PATCH 1/2] SSSDConfig: Port missing parts to python3
* fix incompatible imports
* fix translation.[u]?gettext
* fix dict method has_key
* fix catching multiple exception classes
* fix octal literals PEP 3127
* long is not defined in python3
Resolves:
https://fedorahosted.org/sssd/ticket/2017
---
src/config/SSSDConfig/__init__.py.in | 47 +++++++++++++++++++-----------------
src/config/SSSDConfigTest.py | 24 +++++++++---------
2 files changed, 37 insertions(+), 34 deletions(-)
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index ae00a2b7f9130725a6a766a4cbbba0a53f86dd7a..de03bd6140689c3935d4affcf5fe365b7bc521e7 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -6,9 +6,9 @@ Created on Sep 18, 2009
import os
import gettext
-import exceptions
import re
-from ipachangeconf import SSSDChangeConf
+import sys
+from .ipachangeconf import SSSDChangeConf
# Exceptions
class SSSDConfigException(Exception): pass
@@ -32,7 +32,10 @@ PACKAGE = 'sss_daemon'
LOCALEDIR = '/usr/share/locale'
translation = gettext.translation(PACKAGE, LOCALEDIR, fallback=True)
-_ = translation.ugettext
+if sys.version_info[0] > 2:
+ _ = translation.gettext
+else:
+ _ = translation.ugettext
# TODO: This needs to be made external
option_strings = {
@@ -446,7 +449,7 @@ class SSSDConfigSchema(SSSDChangeConf):
self.type_lookup = {
'bool' : bool,
'int' : int,
- 'long' : long,
+ 'long' : long if sys.version_info[0] == 2 else int,
'float': float,
'str' : str,
'list' : list,
@@ -481,7 +484,7 @@ class SSSDConfigSchema(SSSDChangeConf):
subtype = self.type_lookup[split_option[SUBTYPE]]
mandatory = self.bool_lookup[split_option[MANDATORY]]
- if option_strings.has_key(option['name']):
+ if option['name'] in option_strings:
desc = option_strings[option['name']]
else:
desc = None
@@ -527,7 +530,7 @@ class SSSDConfigSchema(SSSDChangeConf):
mandatory,
desc,
[subtype(split_option[DEFAULT])])
- except ValueError, KeyError:
+ except (ValueError, KeyError):
raise ParsingError
else:
try:
@@ -546,7 +549,7 @@ class SSSDConfigSchema(SSSDChangeConf):
mandatory,
desc,
primarytype(split_option[DEFAULT]))
- except ValueError, KeyError:
+ except (ValueError, KeyError):
raise ParsingError
elif optionlen > 4:
@@ -561,7 +564,7 @@ class SSSDConfigSchema(SSSDChangeConf):
else:
newvalue = subtype(x)
fixed_options.extend([newvalue])
- except ValueError, KeyError:
+ except (ValueError, KeyError):
raise ParsingError
else:
fixed_options.extend([x])
@@ -610,7 +613,7 @@ class SSSDConfigSchema(SSSDChangeConf):
splitsection = section['name'].split('/')
if (splitsection[0] == 'provider'):
if(len(splitsection) == 3):
- if not providers.has_key(splitsection[1]):
+ if splitsection[1] not in providers:
providers[splitsection[1]] = []
providers[splitsection[1]].extend([splitsection[2]])
for key in providers.keys():
@@ -674,7 +677,7 @@ class SSSDConfigObject(object):
=== Errors ===
No errors
"""
- if self.options.has_key(optionname):
+ if optionname in self.options:
del self.options[optionname]
class SSSDService(SSSDConfigObject):
@@ -1309,12 +1312,12 @@ class SSSDDomain(SSSDConfigObject):
# We should now have a list of options used only by this
# provider. So we remove them.
for option in options:
- if self.options.has_key(option):
+ if option in self.options:
del self.options[option]
# Remove this provider from the option list
option = '%s_provider' % provider_type
- if self.options.has_key(option):
+ if option in self.options:
del self.options[option]
self.providers.remove((provider, provider_type))
@@ -1452,10 +1455,10 @@ class SSSDConfig(SSSDChangeConf):
outputfile = self.configfile
# open() will raise IOError if it fails
- old_umask = os.umask(0177)
+ old_umask = os.umask(0o177)
of = open(outputfile, "wb")
output = self.dump(self.opts)
- of.write(output)
+ of.write(output.encode('utf-8'))
of.close()
os.umask(old_umask)
@@ -1477,7 +1480,7 @@ class SSSDConfig(SSSDChangeConf):
if (self.has_option('sssd', 'services')):
active_services = striplist(self.get('sssd', 'services').split(','))
service_dict = dict.fromkeys(active_services)
- if service_dict.has_key(''):
+ if '' in service_dict:
del service_dict['']
# Remove any entries in this list that don't
@@ -1633,7 +1636,7 @@ class SSSDConfig(SSSDChangeConf):
# This guarantees uniqueness and makes it easy
# to add a new value
service_dict = dict.fromkeys(striplist(item['value'].split(',')))
- if service_dict.has_key(''):
+ if '' in service_dict:
del service_dict['']
# Add a new key for the service being activated
@@ -1674,11 +1677,11 @@ class SSSDConfig(SSSDChangeConf):
# This guarantees uniqueness and makes it easy
# to remove the one unwanted value.
service_dict = dict.fromkeys(striplist(item['value'].split(',')))
- if service_dict.has_key(''):
+ if '' in service_dict:
del service_dict['']
# Remove the unwanted service from the lest
- if service_dict.has_key(name):
+ if name in service_dict:
del service_dict[name]
# Write out the joined keys
@@ -1760,7 +1763,7 @@ class SSSDConfig(SSSDChangeConf):
if (self.has_option('sssd', 'domains')):
active_domains = striplist(self.get('sssd', 'domains').split(','))
domain_dict = dict.fromkeys(active_domains)
- if domain_dict.has_key(''):
+ if '' in domain_dict:
del domain_dict['']
# Remove any entries in this list that don't
@@ -1955,7 +1958,7 @@ class SSSDConfig(SSSDChangeConf):
# This guarantees uniqueness and makes it easy
# to add a new value
domain_dict = dict.fromkeys(striplist(item['value'].split(',')))
- if domain_dict.has_key(''):
+ if '' in domain_dict:
del domain_dict['']
# Add a new key for the domain being activated
@@ -1996,11 +1999,11 @@ class SSSDConfig(SSSDChangeConf):
# This guarantees uniqueness and makes it easy
# to remove the one unwanted value.
domain_dict = dict.fromkeys(striplist(item['value'].split(',')))
- if domain_dict.has_key(''):
+ if '' in domain_dict:
del domain_dict['']
# Remove the unwanted domain from the lest
- if domain_dict.has_key(name):
+ if name in domain_dict:
del domain_dict[name]
# Write out the joined keys
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 5d6662a9ad5d27280bb3f48e94cc0fb071665fd6..4c4af18d35232362d664480912a9cece01ec8a63 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -748,12 +748,12 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
# Ensure that all of the expected defaults are there
for provider in control_provider_dict.keys():
for ptype in control_provider_dict[provider]:
- self.assertTrue(providers.has_key(provider))
+ self.assertTrue(provider in providers)
self.assertTrue(ptype in providers[provider])
for provider in providers.keys():
for ptype in providers[provider]:
- self.assertTrue(control_provider_dict.has_key(provider))
+ self.assertTrue(provider in control_provider_dict)
self.assertTrue(ptype in control_provider_dict[provider])
def testListProviderOptions(self):
@@ -1003,7 +1003,7 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
# Remove the local ID provider and add an LDAP one
# LDAP ID providers can also use the krb5_realm
domain.remove_provider('id')
- self.assertFalse(domain.options.has_key('id_provider'))
+ self.assertFalse('id_provider' in domain.options)
domain.add_provider('ldap', 'id')
@@ -1020,7 +1020,7 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
domain.remove_provider('id')
self.assertEquals(domain.get_option('krb5_realm'),
'EXAMPLE.COM')
- self.assertFalse(domain.options.has_key('ldap_uri'))
+ self.assertFalse('ldap_uri' in domain.options)
# Put the LOCAL provider back
domain.add_provider('local', 'id')
@@ -1028,7 +1028,7 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
# Remove the auth domain and verify that the options
# revert to the backup_list
domain.remove_provider('auth')
- self.assertFalse(domain.options.has_key('auth_provider'))
+ self.assertFalse('auth_provider' in domain.options)
options = domain.list_options()
self.assertTrue(type(options) == dict,
@@ -1047,21 +1047,21 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
option)
# Ensure that the krb5_realm option is now gone
- self.assertFalse(domain.options.has_key('krb5_realm'))
+ self.assertFalse('krb5_realm' in domain.options)
# Test removing nonexistent provider - Real
domain.remove_provider('id')
- self.assertFalse(domain.options.has_key('id_provider'))
+ self.assertFalse('id_provider' in domain.options)
# Test removing nonexistent provider - Bad backend type
# Should pass without complaint
domain.remove_provider('id')
- self.assertFalse(domain.options.has_key('id_provider'))
+ self.assertFalse('id_provider' in domain.options)
# Test removing nonexistent provider - Bad provider type
# Should pass without complaint
domain.remove_provider('nosuchprovider')
- self.assertFalse(domain.options.has_key('nosuchprovider_provider'))
+ self.assertFalse('nosuchprovider_provider' in domain.options)
def testGetOption(self):
domain = SSSDConfig.SSSDDomain('sssd', self.schema)
@@ -1367,7 +1367,7 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
# Positive test - Service with invalid option loads
# but ignores the invalid option
service = sssdconfig.get_service('pam')
- self.assertFalse(service.options.has_key('nosuchoption'))
+ self.assertFalse('nosuchoption' in service.options)
def testNewService(self):
sssdconfig = SSSDConfig.SSSDConfig(srcdir + "/etc/sssd.api.conf",
@@ -1598,13 +1598,13 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
# Expected result: Domain is imported, but does not contain the
# unknown provider entry
domain = sssdconfig.get_domain('INVALIDPROVIDER')
- self.assertFalse(domain.options.has_key('chpass_provider'))
+ self.assertFalse('chpass_provider' in domain.options)
# Positive Test - Domain with unknown option
# Expected result: Domain is imported, but does not contain the
# unknown option entry
domain = sssdconfig.get_domain('INVALIDOPTION')
- self.assertFalse(domain.options.has_key('nosuchoption'))
+ self.assertFalse('nosuchoption' in domain.options)
def testNewDomain(self):
sssdconfig = SSSDConfig.SSSDConfig(srcdir + "/etc/sssd.api.conf",
--
2.1.0
-------------- next part --------------
>From 869274e06376a2fa01c3467515654da8ea42a4a8 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Thu, 29 Jan 2015 10:32:23 +0100
Subject: [PATCH 2/2] sbus_codegen: Port to python3
Resolves:
https://fedorahosted.org/sssd/ticket/2017
---
src/sbus/sbus_codegen | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/sbus/sbus_codegen b/src/sbus/sbus_codegen
index d2de58552b8de5bc6178e7a895d3494515680112..ceaf26c82515ad2741d308e8a7769e09a367ac6e 100755
--- a/src/sbus/sbus_codegen
+++ b/src/sbus/sbus_codegen
@@ -60,14 +60,19 @@
# to generate for a given interface or method. By default the codegen will
# build up a symbol name from the DBus name.
#
+from __future__ import print_function
import optparse
import os
import re
-import StringIO
import sys
import xml.parsers.expat
+if sys.version_info[0] > 2:
+ import io
+else:
+ import StringIO as io
+
# -----------------------------------------------------------------------------
# Objects
@@ -765,13 +770,13 @@ class DBusXMLParser:
self.arg_count = 0
try:
- with open(filename, "r") as f:
+ with open(filename, "rb") as f:
parser.ParseFile(f)
- except DBusXmlException, ex:
+ except DBusXmlException as ex:
ex.line = parser.CurrentLineNumber
ex.file = filename
raise
- except xml.parsers.expat.ExpatError, ex:
+ except xml.parsers.expat.ExpatError as ex:
exc = DBusXmlException(str(ex))
exc.line = ex.lineno
exc.file = filename
@@ -895,11 +900,11 @@ def parse_options():
(options, args) = parser.parse_args()
if not args:
- print >> sys.stderr, "sbus_codegen: no input file specified"
+ print("sbus_codegen: no input file specified", file=sys.stderr)
sys.exit(2)
if options.mode not in ["header", "source"]:
- print >> sys.stderr, "sbus_codegen: specify --mode=header or --mode=source"
+ print("sbus_codegen: specify --mode=header or --mode=source", file=sys.stderr)
return options, args
@@ -907,7 +912,7 @@ def main():
options, args = parse_options()
if options.output:
- sys.stdout = buf = StringIO.StringIO()
+ sys.stdout = buf = io.StringIO()
for filename in args:
parser = DBusXMLParser(filename)
@@ -928,6 +933,6 @@ def main():
if __name__ == "__main__":
try:
main()
- except DBusXmlException, ex:
- print >> sys.stderr, str(ex)
+ except DBusXmlException as ex:
+ print(str(ex), file=sys.stderr)
sys.exit(1)
--
2.1.0
More information about the sssd-devel
mailing list