Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 01/27/2011 01:31 PM, Jakub Hrozek wrote:
On 01/26/2011 09:50 PM, Simo Sorce wrote:
On Mon, 2011-01-24 at 15:51 +0100, Jakub Hrozek wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 01/21/2011 05:54 PM, Rob Crittenden wrote:
Jakub Hrozek wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 01/20/2011 11:53 PM, Simo Sorce wrote:
On Thu, 20 Jan 2011 17:27:37 -0500
Dmitri Pald...@redhat.com wrote:
Michael Gregg wrote:
Jakub Hrozek wrote:
Hi,
as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019
to delete a DNS RR one has to remove its record types one by one.
This patch modifies the behaviour so that if the user runs
dnsrecord-delzone record-name with no other parameters, the
whole record is removed.
Alternative solutions might be to expose the internal command that
is able to delete the record (although I think it is
counterintuitive to have one command to remove record types and one
for the whole record) or have a special flag (--del-all?) to remove
the whole record.
The patch also fixes the unit tests as they didn't reflect all the
recent changes.
Going with this patch sounds good, but to make sure, I polled
several
people here, and they all seemed to think that having to add a
--del-all or --del-record flag at the end would be better as it would
be less prone to failure where admins would accidentally delete a
entire record because they didn't specify anything after the zone
record
So, maybe we do need a --del-all or --del-record operator.
Agree.
+1
Someone may simply push enter accidentally while checking what to write
after the command. It would be rather unfortunate.
Simo.
Attached is a new version of the patch that implements --del-all. It
also reports failure when deleting a nonexistent RR (new ticket 829).
nack, this isn't working properly for me.
Here is how I tested:
- add a new zone, newzone1
- ipa dnsrecord-add newzone1 as --a-rec 3.4.5.6
- ipa dnsrecord-add newzone1 as
Record name: as
A record: 3.4.5.6
- ipa dnsrecord-show newzone1 as
Record name: as
A record: 3.4.5.6
- ipa dnsrecord-del newzone1 as --del-all
[ no output ]
- ipa dnsrecord-show newzone1 as
ipa: ERROR: as: DNS resource record not found
So a couple of problems:
1. An error should have been thrown when I tried a delete without a
specific record type.
I agree but I was reluctant to do this because it was perfectly OK to
call dnsrecord-add with no options. That would create an empty DNS
record. The interface was orthogonal so dnsrecord-del with no options
would remove the record if it was empty. But I don't think an empty DNS
record makes any sense.
I changed the behaviour such that:
* dnsrecord-add with no attributes is no longer allowed. You have to
specify at least one RR type.
Apparently this is not effective, I was able to add an empty DNS
record.
Thanks for catching this. A fixed patch is attached.
Apparently the fix was not enough, I got fooled by one callback in the
framework and it worked only by accident. Thanks for testing, Simo.
New patch attached.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk1C6o0ACgkQHsardTLnvCU1xQCg6agQW+AvHRDgPsClnHiPlPNV
gS0AoOU4CsXgRGJ7gnNXivwMGZRw2ORA
=DbJb
-END PGP SIGNATURE-
From f252c185b90091fab9c1ac6a0f67973543d22589 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek jhro...@redhat.com
Date: Thu, 20 Jan 2011 07:54:14 -0500
Subject: [PATCH] Delete the whole DNS record with no parameters
Error out when deleting a nonexistent DNS record
Also fixes the DNS unit tests.
https://fedorahosted.org/freeipa/ticket/816
https://fedorahosted.org/freeipa/ticket/829
---
API.txt |3 +-
ipalib/plugins/dns.py| 52 +++--
tests/test_xmlrpc/test_dns_plugin.py | 38 +---
3 files changed, 71 insertions(+), 22 deletions(-)
diff --git a/API.txt b/API.txt
index 42ba61f..4b84a2d 100644
--- a/API.txt
+++ b/API.txt
@@ -580,9 +580,10 @@ output: Output('summary', (type 'unicode', type 'NoneType'), 'User-friendly
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('value', type 'unicode', The primary_key value of the entry, e.g. 'jdoe' for a user)
command: dnsrecord_del
-args: 2,41,3
+args: 2,42,3
arg: Str('dnszoneidnsname', cli_name='dnszone', label=Gettext('Zone name', domain='ipa', localedir=None), query=True, required=True)
arg: Str('idnsname', attribute=True, cli_name='name', label=Gettext('Record name', domain='ipa', localedir=None), multivalue=False, primary_key=True, query=True, required=True)
+option: Flag('del_all', autofill=True, default=False, label=Gettext('Delete all associated records', domain='ipa', localedir=None))
option: Flag('all',
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
On Fri, 28 Jan 2011 17:10:53 +0100 Jakub Hrozek jhro...@redhat.com wrote: Apparently the fix was not enough, I got fooled by one callback in the framework and it worked only by accident. Thanks for testing, Simo. New patch attached. Ack pushed to master, Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 01/26/2011 09:50 PM, Simo Sorce wrote:
On Mon, 2011-01-24 at 15:51 +0100, Jakub Hrozek wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 01/21/2011 05:54 PM, Rob Crittenden wrote:
Jakub Hrozek wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 01/20/2011 11:53 PM, Simo Sorce wrote:
On Thu, 20 Jan 2011 17:27:37 -0500
Dmitri Pald...@redhat.com wrote:
Michael Gregg wrote:
Jakub Hrozek wrote:
Hi,
as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019
to delete a DNS RR one has to remove its record types one by one.
This patch modifies the behaviour so that if the user runs
dnsrecord-delzone record-name with no other parameters, the
whole record is removed.
Alternative solutions might be to expose the internal command that
is able to delete the record (although I think it is
counterintuitive to have one command to remove record types and one
for the whole record) or have a special flag (--del-all?) to remove
the whole record.
The patch also fixes the unit tests as they didn't reflect all the
recent changes.
Going with this patch sounds good, but to make sure, I polled
several
people here, and they all seemed to think that having to add a
--del-all or --del-record flag at the end would be better as it would
be less prone to failure where admins would accidentally delete a
entire record because they didn't specify anything after the zone
record
So, maybe we do need a --del-all or --del-record operator.
Agree.
+1
Someone may simply push enter accidentally while checking what to write
after the command. It would be rather unfortunate.
Simo.
Attached is a new version of the patch that implements --del-all. It
also reports failure when deleting a nonexistent RR (new ticket 829).
nack, this isn't working properly for me.
Here is how I tested:
- add a new zone, newzone1
- ipa dnsrecord-add newzone1 as --a-rec 3.4.5.6
- ipa dnsrecord-add newzone1 as
Record name: as
A record: 3.4.5.6
- ipa dnsrecord-show newzone1 as
Record name: as
A record: 3.4.5.6
- ipa dnsrecord-del newzone1 as --del-all
[ no output ]
- ipa dnsrecord-show newzone1 as
ipa: ERROR: as: DNS resource record not found
So a couple of problems:
1. An error should have been thrown when I tried a delete without a
specific record type.
I agree but I was reluctant to do this because it was perfectly OK to
call dnsrecord-add with no options. That would create an empty DNS
record. The interface was orthogonal so dnsrecord-del with no options
would remove the record if it was empty. But I don't think an empty DNS
record makes any sense.
I changed the behaviour such that:
* dnsrecord-add with no attributes is no longer allowed. You have to
specify at least one RR type.
Apparently this is not effective, I was able to add an empty DNS
record.
Thanks for catching this. A fixed patch is attached.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk1BZY8ACgkQHsardTLnvCXfwwCgqQDrT6ZwZw20gNM+v+iT0QK5
1gIAoMyIS40UyS4X6VpqPB90U2PiNeLl
=w7gG
-END PGP SIGNATURE-
From e9a0cb849681bb97e0dc5872f977b23a945e2736 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek jhro...@redhat.com
Date: Thu, 20 Jan 2011 07:54:14 -0500
Subject: [PATCH] Delete the whole DNS record with no parameters
Error out when deleting a nonexistent DNS record
Also fixes the DNS unit tests.
https://fedorahosted.org/freeipa/ticket/816
https://fedorahosted.org/freeipa/ticket/829
---
API.txt |3 +-
ipalib/plugins/dns.py| 51 +++--
tests/test_xmlrpc/test_dns_plugin.py | 38 ++---
3 files changed, 70 insertions(+), 22 deletions(-)
diff --git a/API.txt b/API.txt
index 9717acc..c9a56f6 100644
--- a/API.txt
+++ b/API.txt
@@ -580,9 +580,10 @@ output: Output('summary', (type 'unicode', type 'NoneType'), 'User-friendly
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('value', type 'unicode', The primary_key value of the entry, e.g. 'jdoe' for a user)
command: dnsrecord_del
-args: 2,41,3
+args: 2,42,3
arg: Str('dnszoneidnsname', cli_name='dnszone', label=Gettext('Zone name', domain='ipa', localedir=None), query=True, required=True)
arg: Str('idnsname', attribute=True, cli_name='name', label=Gettext('Record name', domain='ipa', localedir=None), multivalue=False, primary_key=True, query=True, required=True)
+option: Flag('del_all', autofill=True, default=False, label=Gettext('Delete all associated records', domain='ipa', localedir=None))
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui', flags=['no_output'])
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui', flags=['no_output'])
option: Str('version?',
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
On Mon, 2011-01-24 at 15:51 +0100, Jakub Hrozek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01/21/2011 05:54 PM, Rob Crittenden wrote: Jakub Hrozek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01/20/2011 11:53 PM, Simo Sorce wrote: On Thu, 20 Jan 2011 17:27:37 -0500 Dmitri Pald...@redhat.com wrote: Michael Gregg wrote: Jakub Hrozek wrote: Hi, as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019 to delete a DNS RR one has to remove its record types one by one. This patch modifies the behaviour so that if the user runs dnsrecord-delzone record-name with no other parameters, the whole record is removed. Alternative solutions might be to expose the internal command that is able to delete the record (although I think it is counterintuitive to have one command to remove record types and one for the whole record) or have a special flag (--del-all?) to remove the whole record. The patch also fixes the unit tests as they didn't reflect all the recent changes. Going with this patch sounds good, but to make sure, I polled several people here, and they all seemed to think that having to add a --del-all or --del-record flag at the end would be better as it would be less prone to failure where admins would accidentally delete a entire record because they didn't specify anything after the zone record So, maybe we do need a --del-all or --del-record operator. Agree. +1 Someone may simply push enter accidentally while checking what to write after the command. It would be rather unfortunate. Simo. Attached is a new version of the patch that implements --del-all. It also reports failure when deleting a nonexistent RR (new ticket 829). nack, this isn't working properly for me. Here is how I tested: - add a new zone, newzone1 - ipa dnsrecord-add newzone1 as --a-rec 3.4.5.6 - ipa dnsrecord-add newzone1 as Record name: as A record: 3.4.5.6 - ipa dnsrecord-show newzone1 as Record name: as A record: 3.4.5.6 - ipa dnsrecord-del newzone1 as --del-all [ no output ] - ipa dnsrecord-show newzone1 as ipa: ERROR: as: DNS resource record not found So a couple of problems: 1. An error should have been thrown when I tried a delete without a specific record type. I agree but I was reluctant to do this because it was perfectly OK to call dnsrecord-add with no options. That would create an empty DNS record. The interface was orthogonal so dnsrecord-del with no options would remove the record if it was empty. But I don't think an empty DNS record makes any sense. I changed the behaviour such that: * dnsrecord-add with no attributes is no longer allowed. You have to specify at least one RR type. Apparently this is not effective, I was able to add an empty DNS record. * dnsrecord-del with no attributes is no longer allowed. You have to either specify a RR type or --del-all. This one tested right. 2. Some output should be displayed when I delete all records, at least a summary. Agreed and fixed. This also checks out. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01/22/2011 02:28 AM, Adam Young wrote: Does any of this imply that we shopuld change the WebUI handling of Zone or Record deletes? Sorry, I don't know enough about the WebUI to give an authoritative answer. I'll try to summarize the changes I did, if it doesn't answer your question, please catch me on IRC :-) The only change to the API is a new option del_all that specifies that the caller wants to delete the whole DNS record. Calling dnsrecord-add and dnsrecord-del with no options is now disallowed. See my reply to Rob's email for more details. The return value of dnsrecord-del changed for the case the whole record is deleted - now it returns the same value other -del commands do, which in the Python CLI world is a dictionary that contains entries we failed to delete. Jakub -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk09kfcACgkQHsardTLnvCXklgCg0qCPgt3RLKOjExvR0HcD/bgN Uo4AmgJkeLFBhKFfMV/2tnmjkrgGYtqY =uN9v -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
On 01/24/2011 09:51 AM, Jakub Hrozek wrote: Sorry, I don't know enough about the WebUI to give an authoritative answer. I'll try to summarize the changes I did, if it doesn't answer your question, please catch me on IRC:-) The only change to the API is a new option del_all that specifies that the caller wants to delete the whole DNS record. Calling dnsrecord-add and dnsrecord-del with no options is now disallowed. See my reply to Rob's email for more details. The return value of dnsrecord-del changed for the case the whole record is deleted - now it returns the same value other -del commands do, which in the Python CLI world is a dictionary that contains entries we failed to delete. I think that this won't change anything UI based. If you want to delete all of the records for a given Zone, you would just select all of them in the UI, so it would be an exhaustive list. To select them all, we have UI control that toggles all of the checkmarks. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 01/20/2011 11:53 PM, Simo Sorce wrote:
On Thu, 20 Jan 2011 17:27:37 -0500
Dmitri Pal d...@redhat.com wrote:
Michael Gregg wrote:
Jakub Hrozek wrote:
Hi,
as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019
to delete a DNS RR one has to remove its record types one by one.
This patch modifies the behaviour so that if the user runs
dnsrecord-del zone record-name with no other parameters, the
whole record is removed.
Alternative solutions might be to expose the internal command that
is able to delete the record (although I think it is
counterintuitive to have one command to remove record types and one
for the whole record) or have a special flag (--del-all?) to remove
the whole record.
The patch also fixes the unit tests as they didn't reflect all the
recent changes.
Going with this patch sounds good, but to make sure, I polled
several
people here, and they all seemed to think that having to add a
--del-all or --del-record flag at the end would be better as it would
be less prone to failure where admins would accidentally delete a
entire record because they didn't specify anything after the zone
record
So, maybe we do need a --del-all or --del-record operator.
Agree.
+1
Someone may simply push enter accidentally while checking what to write
after the command. It would be rather unfortunate.
Simo.
Attached is a new version of the patch that implements --del-all. It
also reports failure when deleting a nonexistent RR (new ticket 829).
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk05oPgACgkQHsardTLnvCUqNwCfQ1BOTgx7L2lVcG5a7a6oP8lW
sX4AoIDw5xVidcKblzWueO5OwlzkZ6kZ
=YkBR
-END PGP SIGNATURE-
From b34a673c3015dd562a96d7f30973fa10a91b7d8d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek jhro...@redhat.com
Date: Thu, 20 Jan 2011 07:54:14 -0500
Subject: [PATCH] Delete the whole DNS record with no parameters
Error out when deleting a nonexistent DNS record
Also fixes the DNS unit tests.
https://fedorahosted.org/freeipa/ticket/816
https://fedorahosted.org/freeipa/ticket/829
---
ipalib/plugins/dns.py| 22 +-
tests/test_xmlrpc/test_dns_plugin.py | 32 ++--
2 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 5b7eb55..900db0e 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -380,6 +380,13 @@ class dnsrecord(LDAPObject):
return self.api.Object[self.parent_object].get_dn(*keys[:-1], **options)
return super(dnsrecord, self).get_dn(*keys, **options)
+def attr_to_cli(self, attr):
+try:
+cliname = attr[:-len('record')].upper()
+except IndexError:
+cliname = attr
+return cliname
+
api.register(dnsrecord)
@@ -518,6 +525,18 @@ class dnsrecord_del(dnsrecord_mod_record):
Delete DNS resource record.
+takes_options = (
+Flag('del_all',
+default=False,
+label=_('Delete all associated records'),
+),
+)
+
+def execute(self, *keys, **options):
+if options.get('del_all', False):
+return self.obj.methods.delentry(*keys)
+return super(dnsrecord_del, self).execute(*keys, **options)
+
def update_old_entry_callback(self, entry_attrs, old_entry_attrs):
for (a, v) in entry_attrs.iteritems():
if not isinstance(v, (list, tuple)):
@@ -526,7 +545,8 @@ class dnsrecord_del(dnsrecord_mod_record):
try:
old_entry_attrs[a].remove(val)
except (KeyError, ValueError):
-pass
+raise errors.NotFound(reason='%s record with value %s not found' %
+ (self.obj.attr_to_cli(a), val))
def post_callback(self, keys, entry_attrs):
if not self.obj.is_pkey_zone_record(*keys):
diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py
index 5fb08fd..6282e95 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -86,7 +86,8 @@ class test_dns(Declarative):
'dn': u'idnsname=%s,cn=dns,%s' % (dnszone1, api.env.basedn),
'idnsname': [dnszone1],
'idnszoneactive': [u'TRUE'],
-'idnssoamname': [u'ns1.%s' % dnszone1],
+'idnssoamname': [u'ns1.%s.' % dnszone1],
+'nsrecord': [u'ns1.%s.' % dnszone1],
'idnssoarname': [u'root.%s.' % dnszone1],
'idnssoaserial': [fuzzy_digits],
'idnssoarefresh': [fuzzy_digits],
@@ -122,7 +123,8 @@ class test_dns(Declarative):
'dn': u'idnsname=%s,cn=dns,%s' %
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
Jakub Hrozek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01/20/2011 11:53 PM, Simo Sorce wrote: On Thu, 20 Jan 2011 17:27:37 -0500 Dmitri Pald...@redhat.com wrote: Michael Gregg wrote: Jakub Hrozek wrote: Hi, as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019 to delete a DNS RR one has to remove its record types one by one. This patch modifies the behaviour so that if the user runs dnsrecord-delzone record-name with no other parameters, the whole record is removed. Alternative solutions might be to expose the internal command that is able to delete the record (although I think it is counterintuitive to have one command to remove record types and one for the whole record) or have a special flag (--del-all?) to remove the whole record. The patch also fixes the unit tests as they didn't reflect all the recent changes. Going with this patch sounds good, but to make sure, I polled several people here, and they all seemed to think that having to add a --del-all or --del-record flag at the end would be better as it would be less prone to failure where admins would accidentally delete a entire record because they didn't specify anything after the zone record So, maybe we do need a --del-all or --del-record operator. Agree. +1 Someone may simply push enter accidentally while checking what to write after the command. It would be rather unfortunate. Simo. Attached is a new version of the patch that implements --del-all. It also reports failure when deleting a nonexistent RR (new ticket 829). nack, this isn't working properly for me. Here is how I tested: - add a new zone, newzone1 - ipa dnsrecord-add newzone1 as --a-rec 3.4.5.6 - ipa dnsrecord-add newzone1 as Record name: as A record: 3.4.5.6 - ipa dnsrecord-show newzone1 as Record name: as A record: 3.4.5.6 - ipa dnsrecord-del newzone1 as --del-all [ no output ] - ipa dnsrecord-show newzone1 as ipa: ERROR: as: DNS resource record not found So a couple of problems: 1. An error should have been thrown when I tried a delete without a specific record type. 2. Some output should be displayed when I delete all records, at least a summary. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
On 01/21/2011 11:54 AM, Rob Crittenden wrote: Jakub Hrozek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01/20/2011 11:53 PM, Simo Sorce wrote: On Thu, 20 Jan 2011 17:27:37 -0500 Dmitri Pald...@redhat.com wrote: Michael Gregg wrote: Jakub Hrozek wrote: Hi, as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019 to delete a DNS RR one has to remove its record types one by one. This patch modifies the behaviour so that if the user runs dnsrecord-delzone record-name with no other parameters, the whole record is removed. Alternative solutions might be to expose the internal command that is able to delete the record (although I think it is counterintuitive to have one command to remove record types and one for the whole record) or have a special flag (--del-all?) to remove the whole record. The patch also fixes the unit tests as they didn't reflect all the recent changes. Going with this patch sounds good, but to make sure, I polled several people here, and they all seemed to think that having to add a --del-all or --del-record flag at the end would be better as it would be less prone to failure where admins would accidentally delete a entire record because they didn't specify anything after the zone record So, maybe we do need a --del-all or --del-record operator. Agree. +1 Someone may simply push enter accidentally while checking what to write after the command. It would be rather unfortunate. Simo. Attached is a new version of the patch that implements --del-all. It also reports failure when deleting a nonexistent RR (new ticket 829). Does any of this imply that we shopuld change the WebUI handling of Zone or Record deletes? nack, this isn't working properly for me. Here is how I tested: - add a new zone, newzone1 - ipa dnsrecord-add newzone1 as --a-rec 3.4.5.6 - ipa dnsrecord-add newzone1 as Record name: as A record: 3.4.5.6 - ipa dnsrecord-show newzone1 as Record name: as A record: 3.4.5.6 - ipa dnsrecord-del newzone1 as --del-all [ no output ] - ipa dnsrecord-show newzone1 as ipa: ERROR: as: DNS resource record not found So a couple of problems: 1. An error should have been thrown when I tried a delete without a specific record type. 2. Some output should be displayed when I delete all records, at least a summary. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
Jakub Hrozek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019 to delete a DNS RR one has to remove its record types one by one. This patch modifies the behaviour so that if the user runs dnsrecord-del zone record-name with no other parameters, the whole record is removed. Alternative solutions might be to expose the internal command that is able to delete the record (although I think it is counterintuitive to have one command to remove record types and one for the whole record) or have a special flag (--del-all?) to remove the whole record. The patch also fixes the unit tests as they didn't reflect all the recent changes. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk04Ml4ACgkQHsardTLnvCWk3wCZAYEuhUBs3dX5RkBiCvsD/Iev VcgAoJzk5cCgzmhityA56g830wNnkaxE =f60L -END PGP SIGNATURE- Going with this patch sounds good, but to make sure, I polled several people here, and they all seemed to think that having to add a --del-all or --del-record flag at the end would be better as it would be less prone to failure where admins would accidentally delete a entire record because they didn't specify anything after the zone record So, maybe we do need a --del-all or --del-record operator. Michael- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
Michael Gregg wrote: Jakub Hrozek wrote: Hi, as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019 to delete a DNS RR one has to remove its record types one by one. This patch modifies the behaviour so that if the user runs dnsrecord-del zone record-name with no other parameters, the whole record is removed. Alternative solutions might be to expose the internal command that is able to delete the record (although I think it is counterintuitive to have one command to remove record types and one for the whole record) or have a special flag (--del-all?) to remove the whole record. The patch also fixes the unit tests as they didn't reflect all the recent changes. Going with this patch sounds good, but to make sure, I polled several people here, and they all seemed to think that having to add a --del-all or --del-record flag at the end would be better as it would be less prone to failure where admins would accidentally delete a entire record because they didn't specify anything after the zone record So, maybe we do need a --del-all or --del-record operator. Agree. Michael- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Thank you, Dmitri Pal Sr. Engineering Manager IPA project, Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters
On Thu, 20 Jan 2011 17:27:37 -0500 Dmitri Pal d...@redhat.com wrote: Michael Gregg wrote: Jakub Hrozek wrote: Hi, as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019 to delete a DNS RR one has to remove its record types one by one. This patch modifies the behaviour so that if the user runs dnsrecord-del zone record-name with no other parameters, the whole record is removed. Alternative solutions might be to expose the internal command that is able to delete the record (although I think it is counterintuitive to have one command to remove record types and one for the whole record) or have a special flag (--del-all?) to remove the whole record. The patch also fixes the unit tests as they didn't reflect all the recent changes. Going with this patch sounds good, but to make sure, I polled several people here, and they all seemed to think that having to add a --del-all or --del-record flag at the end would be better as it would be less prone to failure where admins would accidentally delete a entire record because they didn't specify anything after the zone record So, maybe we do need a --del-all or --del-record operator. Agree. +1 Someone may simply push enter accidentally while checking what to write after the command. It would be rather unfortunate. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
