Re: [Freeipa-devel] [PATCH] 0001 Enhance the DNSNotARecordError message
On Mon, Jul 13, 2015 at 03:12:13PM +0200, Petr Spacek wrote: Personally-opinionated-NACK. I would like to avoid advertising --force options when possible. --force should not be necessary in proper setups and advertising it will make people to use it instead of fixing underlying problems. How do you propose for things to work when the host is pre-created (with --random) and the service should be pre-created, and then IP address will only be set by the machine itself when it IPA-enrolls with the OTP? Can we *please* drop this patch? Does your nack go against this patch (code change), or against the ticket https://fedorahosted.org/freeipa/ticket/3959 itself? Frankly, I don't really understand why service-add checks for the DNS record at all. DNS is a property of host, not service. Yes, it might be nice to advise the user that they do not have DNS record for the host but the current ipa: ERROR: Host does not have corresponding DNS A record is just bad user experience. Do you propose to change that ERROR to warning, for example, relaxing the requirement for the DNS records being present? -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat -- 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
Re: [Freeipa-devel] [PATCH] 0001 Enhance the DNSNotARecordError message
On Mon, 13 Jul 2015, Jan Pazdziora wrote: On Mon, Jul 13, 2015 at 03:12:13PM +0200, Petr Spacek wrote: Personally-opinionated-NACK. I would like to avoid advertising --force options when possible. --force should not be necessary in proper setups and advertising it will make people to use it instead of fixing underlying problems. How do you propose for things to work when the host is pre-created (with --random) and the service should be pre-created, and then IP address will only be set by the machine itself when it IPA-enrolls with the OTP? This is a workflow question, not a code fix. If you need to use --force, use it but this specific flow has to be documented, not suggested by the code. We have plenty of cases where you have to use --addattr/--setattr as well, but we don't advertise them in the error messages. On contrary, documenting the fact that in some workflows you actually need to override default belts and suspenders is fine. -- / Alexander Bokovoy -- 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
Re: [Freeipa-devel] [PATCH] 0001 Enhance the DNSNotARecordError message
On 13.7.2015 16:32, Alexander Bokovoy wrote: On Mon, 13 Jul 2015, Jan Pazdziora wrote: On Mon, Jul 13, 2015 at 03:12:13PM +0200, Petr Spacek wrote: Personally-opinionated-NACK. I would like to avoid advertising --force options when possible. --force should not be necessary in proper setups and advertising it will make people to use it instead of fixing underlying problems. How do you propose for things to work when the host is pre-created (with --random) and the service should be pre-created, and then IP address will only be set by the machine itself when it IPA-enrolls with the OTP? This is a workflow question, not a code fix. If you need to use --force, use it but this specific flow has to be documented, not suggested by the code. We have plenty of cases where you have to use --addattr/--setattr as well, but we don't advertise them in the error messages. On contrary, documenting the fact that in some workflows you actually need to override default belts and suspenders is fine. I agree with Alexander. The point is that you have to know what you are doing if you decide to use --force/--setattr and advertising them will lead to cargo cults. The idea of services/hosts without host entry may be worth discussing, please start a separate thread on ipa-devel. -- Petr^2 Spacek -- 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
Re: [Freeipa-devel] [PATCH] 0001 Enhance the DNSNotARecordError message
On 10.7.2015 20:06, Veronika Kabatova wrote: - Original Message - From: Tomas Babej tba...@redhat.com To: Veronika Kabatova vkaba...@redhat.com, freeipa-devel@redhat.com Sent: Friday, July 10, 2015 2:56:58 PM Subject: Re: [Freeipa-devel] [PATCH] 0001 Enhance the DNSNotARecordError message On 07/09/2015 01:49 PM, Veronika Kabatova wrote: The attached patch solves the https://fedorahosted.org/freeipa/ticket/3959 ticket. Veronika Kabatova Hello, thanks for the patch. Actually, the doctest does not pass: $ ipa-run-tests /usr/lib/python2.7/site-packages/ipalib/errors.py --doctest-modules = test session starts = platform linux2 -- Python 2.7.10 -- py-1.4.28 -- pytest-2.6.4 plugins: multihost, sourceorder collected 85 items ../ipalib/errors.py ...F..F.. == FAILURES === _ [doctest] ipalib.errors.DNSNotARecordError __ 1137 1138 **4019** Raised when a hostname is not a DNS A/ record 1139 1140 For example: 1141 1142 raise DNSNotARecordError() Differences (unified diff with -expected +actual): @@ -1,4 +1,6 @@ Traceback (most recent call last): - ... -DNSNotARecordError: Host does not have corresponding DNS A/ record, -use --force to continue anyway + File /usr/lib64/python2.7/doctest.py, line 1315, in __run +compileflags, 1) in test.globs + File doctest ipalib.errors.DNSNotARecordError[0], line 1, in module +raise DNSNotARecordError() +DNSNotARecordError: Host does not have corresponding DNS A/ record, use --force to continue anyway /usr/lib/python2.7/site-packages/ipalib/errors.py:1142: DocTestFailure The reason for the mismatch here is that you wrapped the line - in this case, we need to violate the PEP8, and allow the length of the line exceed 80 characters. Good to know, thanks for clarifying. Attached modified version which doesn't break tests, even if PEP8 checker is not happy with it. Personally-opinionated-NACK. I would like to avoid advertising --force options when possible. --force should not be necessary in proper setups and advertising it will make people to use it instead of fixing underlying problems. Can we *please* drop this patch? -- Petr^2 Spacek -- 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
Re: [Freeipa-devel] [PATCH] 0001 Enhance the DNSNotARecordError message
On 07/09/2015 01:49 PM, Veronika Kabatova wrote: The attached patch solves the https://fedorahosted.org/freeipa/ticket/3959 ticket. Veronika Kabatova Hello, thanks for the patch. Actually, the doctest does not pass: $ ipa-run-tests /usr/lib/python2.7/site-packages/ipalib/errors.py --doctest-modules = test session starts = platform linux2 -- Python 2.7.10 -- py-1.4.28 -- pytest-2.6.4 plugins: multihost, sourceorder collected 85 items ../ipalib/errors.py ...F..F.. == FAILURES === _ [doctest] ipalib.errors.DNSNotARecordError __ 1137 1138 **4019** Raised when a hostname is not a DNS A/ record 1139 1140 For example: 1141 1142 raise DNSNotARecordError() Differences (unified diff with -expected +actual): @@ -1,4 +1,6 @@ Traceback (most recent call last): - ... -DNSNotARecordError: Host does not have corresponding DNS A/ record, -use --force to continue anyway + File /usr/lib64/python2.7/doctest.py, line 1315, in __run +compileflags, 1) in test.globs + File doctest ipalib.errors.DNSNotARecordError[0], line 1, in module +raise DNSNotARecordError() +DNSNotARecordError: Host does not have corresponding DNS A/ record, use --force to continue anyway /usr/lib/python2.7/site-packages/ipalib/errors.py:1142: DocTestFailure The reason for the mismatch here is that you wrapped the line - in this case, we need to violate the PEP8, and allow the length of the line exceed 80 characters. HTH, Tomas -- 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
Re: [Freeipa-devel] [PATCH] 0001 Enhance the DNSNotARecordError message
- Original Message -
From: Tomas Babej tba...@redhat.com
To: Veronika Kabatova vkaba...@redhat.com, freeipa-devel@redhat.com
Sent: Friday, July 10, 2015 2:56:58 PM
Subject: Re: [Freeipa-devel] [PATCH] 0001 Enhance the DNSNotARecordError
message
On 07/09/2015 01:49 PM, Veronika Kabatova wrote:
The attached patch solves the
https://fedorahosted.org/freeipa/ticket/3959 ticket.
Veronika Kabatova
Hello,
thanks for the patch.
Actually, the doctest does not pass:
$ ipa-run-tests /usr/lib/python2.7/site-packages/ipalib/errors.py
--doctest-modules
=
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.28 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 85 items
../ipalib/errors.py
...F..F..
==
FAILURES
===
_
[doctest] ipalib.errors.DNSNotARecordError
__
1137
1138 **4019** Raised when a hostname is not a DNS A/ record
1139
1140 For example:
1141
1142 raise DNSNotARecordError()
Differences (unified diff with -expected +actual):
@@ -1,4 +1,6 @@
Traceback (most recent call last):
- ...
-DNSNotARecordError: Host does not have corresponding DNS A/ record,
-use --force to continue anyway
+ File /usr/lib64/python2.7/doctest.py, line 1315, in __run
+compileflags, 1) in test.globs
+ File doctest ipalib.errors.DNSNotARecordError[0], line 1, in
module
+raise DNSNotARecordError()
+DNSNotARecordError: Host does not have corresponding DNS A/
record, use --force to continue anyway
/usr/lib/python2.7/site-packages/ipalib/errors.py:1142: DocTestFailure
The reason for the mismatch here is that you wrapped the line - in this
case, we need to violate the PEP8, and allow the length of the line
exceed 80 characters.
Good to know, thanks for clarifying. Attached modified version which doesn't
break tests, even if PEP8 checker is not happy with it.
HTH,
Tomas
Thanks,
Veronika KabatovaFrom 2a05588cd8c063838a1bca8fa996fbc141bdaa65 Mon Sep 17 00:00:00 2001
From: Veronika Kabatova vkaba...@redhat.com
Date: Fri, 10 Jul 2015 19:33:58 +0200
Subject: [PATCH] Enhance the DNSNotARecordError message
Enhance the DNSNotARecordError message as proposed in ticket #3959. User
is now suggested to use --force option.
https://fedorahosted.org/freeipa/ticket/3959
---
ipalib/errors.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 7e34a879f1d9fad1ed0cbde263cda5cf6d84b7f9..69476b7706c704ad28b1808b7b80863b71dd775e 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1142,12 +1142,12 @@ class DNSNotARecordError(ExecutionError):
raise DNSNotARecordError()
Traceback (most recent call last):
...
-DNSNotARecordError: Host does not have corresponding DNS A/ record
+DNSNotARecordError: Host does not have corresponding DNS A/ record, use --force to continue anyway
errno = 4019
-format = _('Host does not have corresponding DNS A/ record')
+format = _('Host does not have corresponding DNS A/ record, use --force to continue anyway')
class ManagedGroupError(ExecutionError):
--
2.4.3
--
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
