Hi, here are my comments:
On Thu, Sep 24, 2009 at 09:32:27PM -0400, Stephen Gallagher wrote: > Patch 0001 (sgallagh): Convert the existing options to the new names. --- a/server/providers/krb5/krb5_auth.c +++ b/server/providers/krb5/krb5_auth.c @@ -895,7 +895,7 @@ int sssm_krb5_auth_init(struct be_ctx *bectx, ctx->realm = value; ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path, - CONFDB_KRB5_REALM, "/tmp", &value); + CONFDB_KRB5_CCACHEDIR, "/tmp", &value); if (ret != EOK) goto fail; ret = lstat(value, &stat_buf); if (ret != EOK) { > Patch 0002 (jhrozek): Update the manpages to reflect the new names. - there is too much whitespace at line 771 of the patch - in the example the sssd-krb5 the '[domains] section' is referred to and the example uses [domains/FOO] instead of [domain/FOO]. Maybe it should be underlined that this example is incomplete (missing id_proiver) > Patch 0003 (jhrozek+sgallagh): Create a python script to convert > existing config files to the new format and run it automatically during > %post in the sssd.spec > - is it planned to use upgrade_config.py for future upgrade, too, or is it planned to have separate tools for each upgrade. In the latter case I would call to something like upgrade_config_v1_v2.py - when using '-f /somewhere/my.conf' the backup is created in /somewhere but the new config is written to /etc/sss/sssd.conf. My expectation was that the conversion would be done in-place. If you prefer it this way '--help' should say that the default output file is /etc/sss/sssd.conf - upgrade_config.py has problems with '%' used in the krb5_ccname_template: ERROR: '%' must be followed by '%' or '(', found: '%h/krb5cc_%u_%U_%p_%r_%P_%%_XXXXXX' Traceback (most recent call last): File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 327, in main config.upgrade_v2(options.outfile, options.backup) File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 274, in upgrade_v2 self._migrate_domains() File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 210, in _migrate_domains self._migrate_domain(domain) File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 200, in _migrate_domain self._migrate_kw(new_domsec, old_domsec, krb5_kw) File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 101, in _migrate_kw self._migrate_if_exists(to_section, new, from_section, old) File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 93, in _migrate_if_exists self._config.get(from_section, from_option)) File "/usr/lib/python2.6/ConfigParser.py", line 545, in get return self._interpolate(section, option, value, d) File "/usr/lib/python2.6/ConfigParser.py", line 613, in _interpolate self._interpolate_some(option, L, rawval, section, vars, 1) File "/usr/lib/python2.6/ConfigParser.py", line 655, in _interpolate_some "'%%' must be followed by '%%' or '(', found: %r" % (rest,)) InterpolationSyntaxError: '%' must be followed by '%' or '(', found: '%h/krb5cc_%u_%U_%p_%r_%P_%%_XXXXXX' bye, Sumit > Jakub has reviewed patch 0001 and I have reviewed Patch 0002 and 0003 > (though I made some additional fixes to his version of 0003). I'm > looking for one additional reviewer in order to push these. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel