On (03/09/15 09:35), Jakub Hrozek wrote:
> On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
>> On (01/09/15 12:55), Michal Židek wrote:
>>> On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
>>>> On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
>>>>> On (01/09/15 10:51), Jakub Hrozek wrote:
>>>>>> On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
>>>>>>> On (10/08/15 17:18), Michal Židek wrote:
>>>>>>>> On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
>>>>>>>>> On (05/08/15 13:41), Michal Židek wrote:
>>>>>>>>>> On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
>>>>>>>>>>>>>> - } else if (version <
CONFDB_VERSION_INT) {
>>>>>>>>>>>>>> -
DEBUG(SSSDBG_FATAL_FAILURE,
>>>>>>>>>>>>>> - "Config
file is an old version. "
>>>>>>>>>>>>>> - "Please
run configuration upgrade script.\n");
>>>>>>>>>>>>>> - ret = EINVAL;
>>>>>>>>>>>>>> - goto done;
>>>>>>>>>>>>>> - } else if (version >
CONFDB_VERSION_INT) {
>>>>>>>>>>>>>> -
DEBUG(SSSDBG_FATAL_FAILURE,
>>>>>>>>>>>>>> - "Config
file version is newer than confdb\n");
>>>>>>>>>>>>>> - ret = EINVAL;
>>>>>>>>>>>>>> - goto done;
>>>>>>>>>>>>>> + /* No known version. Use
default. */
>>>>>>>>>>>>>> +
DEBUG(SSSDBG_CONF_SETTINGS,
>>>>>>>>>>>>>> + "Value of
config_file_version option not found. "
>>>>>>>>>>>>>> + "Assumed to
be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
>>>>>>>>>>>>>> + } else {
>>>>>>>>>>>>>> + version =
sss_ini_get_int_config_value(init_data,
>>>>>>>>>>>>>> +
CONFDB_DEFAULT_CFG_FILE_VER,
>>>>>>>>>>>>>> +
-1, &ret);
>>>>>>>>>>>>>> + if (ret != EOK) {
>>>>>>>>>>>>>> +
DEBUG(SSSDBG_FATAL_FAILURE,
>>>>>>>>>>>>>> + "Config
file version could not be determined\n");
>>>>>>>>>>> ^^
>>>>>>>>>>> I do not prefer nested "if"s. If
you decided to do it in this way then
>>>>>>>>>>
>>>>>>>>>> Me neither, but sss_ini_get_int_config_value()
has to be
>>>>>>>>>> skipped conditionally. It is just call to the
function
>>>>>>>>>> plus error checking that is nested. I think it is
not
>>>>>>>>>> too bad in this case.
>>>>>>>>>>
>>>>>>>>>>> you shoudl have proper indentation.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Fixed in the new version.
>>>>>>>>>>
>>>>>>>>> It works because integration tests passed.
>>>>>>>>>
http://sssd-ci.duckdns.org/logs/job/20/68/summary.html
>>>>>>>>> But ...
>>>>>>>>>
>>>>>>>>> I tested new version with ipa-client-install
>>>>>>>>> and "config_file_version = 2" is still
added to sssd.conf
>>>>>>>>> even though it is a default value.
>>>>>>>>>
>>>>>>>>> ipa-client-install uses our python API
(python-sssdconfig)
>>>>>>>>> and it does not try to add this option itself.
>>>>>>>>
>>>>>>>> I often change version of SSSD with git checkout
sssd<version>.
>>>>>>>> If I generate the config with realmd or
ipa-client-install
>>>>>>> realmd do not use python module SSSDConfig.
>>>>>>>
>>>>>>>> with latest version then the config_version_file
>>>>>>>> would need to be added manually (and I am pretty
>>>>>>>> sure it would be after I looked into logs to see why
sssd
>>>>>>>> is not starting). I know this will probably only hit
>>>>>>>> testers/developers, but I would prefer not to add
unnecessary
>>>>>>> just developers and developers useually join machine
>>>>>>> to AD or IPA just once.
>>>>>>>
>>>>>>>> little inconveniences.
>>>>>>>>
>>>>>>> I do not try old sssd version very often. Just in case of
bisect.
>>>>>>> and ther is nice/simple workarount for "little
inconvenience"
>>>>>>>
>>>>>>> Just manually add "config_version_file = 2" to
sssd.conf
>>>>>>>
>>>>>>> The main point of this ticket is to simplify sssd.conf
>>>>>>> and our python sssdconfig API should do the same.
>>>>>>> Otherwise we do not need to do such change.
>>>>>>>
>>>>>>> I still see "config_file_version = 2" in sssd.conf
>>>>>>> It was generated by python module SSSDConfig (via
ipa-client-install)
>>>>>>>
>>>>>>> LS
>>>>>>
>>>>>> This thread has stalled, let's try to restart it.
>>>>>>
>>>>>> What versions are normally still being worked on by developers?
I
>>>>>> suspect it's master and sssd-1-12. What about pushing the
patch to
>>>>>> sssd-1-12 as well?
>>>>> I'm fine with sssd-1-12.
>>>>> The python sssdconfig need to be updated.
>>>>>
>>>>> LS
>>>>
>>>> Michal, if you agree, please update the patches so that we can push them
>>>> in time for sssd-1.13.1
>>>
>>> New patch attached. I tested ipa-client-install and the
>>> config_file_version is no longer added.
>>>
>>> Michal
>>>
>>
>> >From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
>>> Date: Tue, 7 Jul 2015 15:15:32 +0200
>>> Subject: [PATCH] CONFDB: Assume config file version 2 if missing
>>>
>>> Default to config file version 2 if the version
>>> is not specified explicitly.
>>>
>>> Ticket:
>>>
https://fedorahosted.org/sssd/ticket/2688
>>> ---
>> * integration tests passed
>> * ipa-client generated config without config_file_version and sssd worked.
>>
>> ACK
>>
>> LS
>
> * master: 175613be0cfb0890174d12d941e634d833b63dd9
>
> Michal, can you rebase the patch for sssd-1-12 ?
The conflict might me caused by missing integration tests in 1.12
LS