On 22.10.2015 01:28, Gabe Alford wrote:
Thanks Martin^2. Updated patched attached.

On Wed, Oct 21, 2015 at 2:46 AM, Martin Basti <mba...@redhat.com <mailto:mba...@redhat.com>> wrote:



    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
    <redhatri...@gmail.com <mailto:redhatri...@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
        <pspa...@redhat.com <mailto:pspa...@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
            <pspa...@redhat.com <mailto:pspa...@redhat.com>
            >> <mailto:pspa...@redhat.com
            <mailto:pspa...@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)







ACK

Pushed to:
master: a0b8415236addf0ff32b9e05b2491d626d483171
ipa-4-2: 85dc0c2e3a396ff7d8d429a414b717cc01231b26

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to