[Freeipa-devel] [PATCH 0057] Warn in no installation found when running ipa-server-install --uninstall

Martin Basti mbasti at redhat.com
Wed Oct 21 08:46:04 UTC 2015



On 20.10.2015 05:17, Gabe Alford wrote:
> Bump for re-review.

Hello,

thank your for your patch, the patch LGTM, but please use print() as 
function to be python2/3 compatible

Martin^2
>
> On Tue, Oct 13, 2015 at 7:15 AM, Gabe Alford <redhatrises at gmail.com 
> <mailto:redhatrises at gmail.com>> wrote:
>
>     No worries Petr. All a part of the review process.
>
>     I have attached an updated patch that prints only a warning message.
>
>     thanks,
>
>     Gabe
>
>     On Tue, Oct 13, 2015 at 12:39 AM, Petr Spacek <pspacek at redhat.com
>     <mailto:pspacek at redhat.com>> wrote:
>
>         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
>
>         On 13.10.2015 <tel:13.10.2015> 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
>         <pspacek at redhat.com <mailto:pspacek at redhat.com>
>         >> <mailto:pspacek at redhat.com <mailto:pspacek at redhat.com>>> 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)
>
>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151021/6a280625/attachment.htm>


More information about the Freeipa-devel mailing list