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 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). > > > > 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