[Freeipa-devel] [PATCHES 0227-0229] Server upgrade: introduce ipa-server-upgrade command
Martin Basti
mbasti at redhat.com
Tue Apr 21 08:31:16 UTC 2015
On 21/04/15 08:12, Jan Cholasta wrote:
> Hi,
>
> Dne 15.4.2015 v 16:26 Martin Basti napsal(a):
>> https://fedorahosted.org/freeipa/ticket/4904
>>
>> Patches attached.
>>
>> Also ipa-upgradeconfig part is called as a subprocess. This will be
>> removed after installer modifications.
>>
>> This patch may cause temporal upgrade issues (corner cases), until
>> installer part will be finished.
>>
>> If somebody will be hit by them, please use --skip-version-check for
>> ipactl and ipa-server-upgrade.
>
> Regarding that option vs. --force: I think the common assumption is
> that --force ignores *all* non-fatal errors, but you break that
> assumption in ipactl. IMO --force should both ignore errors in service
> startup *and* skip version check, and a new option should be added to
> just ignore errors in service startup (e.g. --ignore-service-failures).
Originally I used --force option to skip detection, but there was
objections against it on list.
However, to have option --force, which set true for both
--ignore-service-failures and --skip-version-check options, might be better.
>
> ipa-server-upgrade should probably also have --force, even if it does
> the same thing as --skip-version-check, again because --force is common.
>
>
> This is a weird API:
>
> + if data_upgrade.badsyntax:
> + raise admintool.ScriptError(
> + 'Bad syntax detected in upgrade file(s).', 1)
> + elif data_upgrade.upgradefailed:
> + raise admintool.ScriptError('IPA upgrade failed.', 1)
> + elif data_upgrade.modified:
> + self.log.info('Data update complete')
> + else:
> + self.log.info('Data update complete, no data were modified')
>
> Why does not IPAUpgrade raise errors instead?
>
For historical reasons, I can investigate what would break this change,
I will send it in separate patch.
>
> +class IPAVersionError(Exception):
> + pass
> +
> +class PlatformMismatchError(IPAVersionError):
> + pass
> +
> +class DataUpgradeRequiredError(IPAVersionError):
> + pass
> +
> +class DataInNewerVersionError(IPAVersionError):
> + pass
>
> I don't like the "IPA" in "IPAVersionError", it does not tell you much
> about what kind of version is that. Also data version errors should
> only tell you what is wrong, not how you fix it. IMO better names for
> these would be e.g. "UpgradeVersionError", "UpgradePlatformError",
> "UpgradeDataOlderVersionError", "UpgradeDataNewerVersionError".
> Similar for store_ipa_version and check_ipa_version.
>
Ok.
>
> Why is it not an error if there is no version in check_ipa_version?
> IMO it should, even if you then ignore the exception most of the time.
I can raise error in that case and ignore the exception.
>
>
> Honza
>
Martin^2
--
Martin Basti
More information about the Freeipa-devel
mailing list