<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 23/04/15 12:55, Martin Basti wrote:<br>
</div>
<blockquote cite="mid:5538CFA9.9080700@redhat.com" type="cite">On
21/04/15 10:31, Martin Basti wrote:
<br>
<blockquote type="cite">On 21/04/15 08:12, Jan Cholasta wrote:
<br>
<blockquote type="cite">Hi,
<br>
<br>
Dne 15.4.2015 v 16:26 Martin Basti napsal(a):
<br>
<blockquote type="cite"><a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/4904">https://fedorahosted.org/freeipa/ticket/4904</a>
<br>
<br>
Patches attached.
<br>
<br>
Also ipa-upgradeconfig part is called as a subprocess. This
will be
<br>
removed after installer modifications.
<br>
<br>
This patch may cause temporal upgrade issues (corner cases),
until
<br>
installer part will be finished.
<br>
<br>
If somebody will be hit by them, please use
--skip-version-check for
<br>
ipactl and ipa-server-upgrade.
<br>
</blockquote>
<br>
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).
<br>
</blockquote>
Originally I used --force option to skip detection, but there
was objections against it on list.
<br>
<br>
However, to have option --force, which set true for both
--ignore-service-failures and --skip-version-check options,
might be better.
<br>
<br>
<blockquote type="cite">
<br>
ipa-server-upgrade should probably also have --force, even if
it does the same thing as --skip-version-check, again because
--force is common.
<br>
<br>
<br>
This is a weird API:
<br>
<br>
+ if data_upgrade.badsyntax:
<br>
+ raise admintool.ScriptError(
<br>
+ 'Bad syntax detected in upgrade file(s).', 1)
<br>
+ elif data_upgrade.upgradefailed:
<br>
+ raise admintool.ScriptError('IPA upgrade
failed.', 1)
<br>
+ elif data_upgrade.modified:
<br>
+ self.log.info('Data update complete')
<br>
+ else:
<br>
+ self.log.info('Data update complete, no data were
modified')
<br>
<br>
Why does not IPAUpgrade raise errors instead?
<br>
<br>
</blockquote>
For historical reasons, I can investigate what would break this
change, I will send it in separate patch.
<br>
<blockquote type="cite">
<br>
+class IPAVersionError(Exception):
<br>
+ pass
<br>
+
<br>
+class PlatformMismatchError(IPAVersionError):
<br>
+ pass
<br>
+
<br>
+class DataUpgradeRequiredError(IPAVersionError):
<br>
+ pass
<br>
+
<br>
+class DataInNewerVersionError(IPAVersionError):
<br>
+ pass
<br>
<br>
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.
<br>
<br>
</blockquote>
Ok.
<br>
<blockquote type="cite">
<br>
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.
<br>
</blockquote>
I can raise error in that case and ignore the exception.
<br>
<blockquote type="cite">
<br>
<br>
Honza
<br>
<br>
</blockquote>
Martin^2
<br>
<br>
</blockquote>
Updated patches attached.
<br>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
Updated patches attached<br>
<br>
<pre class="moz-signature" cols="72">--
Martin Basti</pre>
</body>
</html>