[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion MartinBasti commented: """ Fixed upstream master: https://fedorahosted.org/freeipa/changeset/f51869bf5214e2d2322f85bf72b7ae86b6893974 """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-278655609 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion MartinBasti commented: """ ACK and I found a new bug: https://fedorahosted.org/freeipa/ticket/6654 """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-278649734 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion frasertweedale commented: """ Any other changes requested? What's preventing ack on this? """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-278236565 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion stlaz commented: """ @frasertweedale Alright. I am definitely not against having it separated since we came to the realization that replica install checks can not be merged in a satisfactory way anyway. """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-277604102 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion frasertweedale commented: """ @stlaz there are three considerations when "checking the DL": 1. Retrieving the current DL. 2. Checking that current DL is supported by server version. 3. Checking that attempted method of installation is supported on currently DL. Whether it makes sense to have a unified function for (3), I am not sure. I think the approach as implemented in this PR - that each replica installation method checks the DL and if necessary raises an appropriate error message - is satisfactory. Certainly it makes more sense to me to have these checks separate from the check for (2). """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-277423018 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion stlaz commented: """ The purpose of `check_domain_level()` was to have a unified means of checking whether the domain level in the rest of the domain corresponds to the installation media which is presented by the user. Looking back at it I think I chose poor naming and documentation of the check so it's rather confusing. If you find a way to make a unified check in both domain levels (=> a single function call for both DLs that will raise exception when wrong installation media is presented), that'd be nice. Otherwise feel free to split it back to what it was previously. """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-277263462 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion frasertweedale commented: """ @HonzaCholasta @MartinBasti PR updated. I extracted the specific (== 0) and (>= 1) checks to the relevant call sites. Also separated DL retrieval and "DL in range for IPA version" check into separate functions. """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-276571652 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion frasertweedale commented: """ So, what do we want the behaviour of `check_domain_level` to be? I just want to make a small change so that replica install does not break if DL > 1. """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-276529816 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion MartinBasti commented: """ IMO the whole `check_domain_level` is somehow broken, AFAIK the main purpose of it is to print correct error message related to replica file option, depending on current and expected domain level. @stlaz may know more details """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-276351845 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion HonzaCholasta commented: """ I see. The point is, `check_domain_level()` is supposed to check whether replica promotion is possible or not in the current domain level, so it's weird it has an expected domain level argument and even weirder to introduce additional minimum domain level argument, when all it should have is a single boolean argument saying wheter you want to promote or not: ```python def check_domain_level(api, want_promote): ... promote = current >= constants.DOMAIN_LEVEL_1 if promote != want_promote: raise RuntimeError(message) ... ``` """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-276334410 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion MartinBasti commented: """ expected is for domain level 0, because there are different expectations about replica file, it must exactly match domain level 0, you cannot have higher DL. """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-276319884 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion HonzaCholasta commented: """ Excuse me, but what is the point of checking for an exact domain level? Shouldn't `check_domain_level()` rather always check for a minimum domain level? """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-276308946 -- 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
[Freeipa-devel] [freeipa PR#416][comment] replica install: relax domain level check for promotion
URL: https://github.com/freeipa/freeipa/pull/416 Title: #416: replica install: relax domain level check for promotion frasertweedale commented: """ Build failure is unrelated to patch. """ See the full comment at https://github.com/freeipa/freeipa/pull/416#issuecomment-275988778 -- 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