<div dir="ltr"><div><div><div>No worries Petr. All a part of the review process. </div>I have attached an updated patch that prints only a warning message. </div>thanks, </div>Gabe </div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Oct 13, 2015 at 12:39 AM, Petr Spacek <<a href="mailto:pspacek@redhat.com" target="_blank">pspacek@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello Gabe, I would like to apologize for the confusion regarding this patch and the repeated reworking. Unfortunately Honza's position is not mentioned in the ticket so you could not know what to do, but Honza is our "installer architect" so he has final say. Petr^2 Spacek <div class="HOEnZb"><div class="h5"> On <a href="tel:13.10.2015" value="+13102015">13.10.2015</a> 08:31, Jan Cholasta wrote: > Hi, > > I don't think this is the correct approach. We are aiming to have idempotent > installers, which means that running uninstall on a system without IPA > installed should be a no-op. This is the current behavior, so your patch is > actually moving us back. > > The proper fix would be to *remove* the check from install (as opposed to > adding it to uninstall), but this requires the install code to be idempotent, > and we're not there yet. > > I'm OK with making this a warning, but don't make it a fatal error and/or > require --force. > > Honza > > On 12.10.2015 17:12, Gabe Alford wrote: >> Thanks, Petr. Updated patch attached. >> >> Gabe >> >> On Mon, Oct 12, 2015 at 12:47 AM, Petr Spacek <<a href="mailto:pspacek@redhat.com">pspacek@redhat.com</a> >> <mailto:<a href="mailto:pspacek@redhat.com">pspacek@redhat.com</a>>> wrote: >> >> Hello Gabe, >> >> thank you for your patch! >> >> Please note that there might be a case where detection >> is_ipa_configured() is >> broken but the user still needs to run the uninstall process to >> clean it up. >> >> Could you amend the patch to respect --force option? In that case the >> detection should be skipped. >> >> Thank you for your time! >> >> Petr^2 Spacek >> >> On 9.10.2015 19:17, Gabe Alford wrote: >> > diff --git a/ipaserver/install/server/install.py >> b/ipaserver/install/server/install.py >> > index >> >> 13a59a0e6149dc22ded4a895db02516e9360e02b..ca93e7a6fd7276d9c0d82eb6f94575730759d858 >> >> 100644 >> > --- a/ipaserver/install/server/install.py >> > +++ b/ipaserver/install/server/install.py >> > @@ -954,6 +954,12 @@ def uninstall_check(installer): >> > >> > installer._installation_cleanup = False >> > >> > + if not is_ipa_configured(): >> > + print("IPA server is not configured on this system.\n" + >> > + "If you want to install the IPA server, please >> install " + >> > + "it using 'ipa-server-install'.") >> > + sys.exit(1) >> > + >> > fstore = sysrestore.FileStore(SYSRESTORE_DIR_PATH) >> > sstore = sysrestore.StateFile(SYSRESTORE_DIR_PATH) </div></div></blockquote></div> </div>