From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rymes To: development@lists.ipfire.org Subject: Re: Aw: Re: Re: [PATCH] Fix for Bug #12050: Adding fixed leases with one 'add' click / Bug #10629 Date: Mon, 20 May 2019 12:49:27 -0400 Message-ID: <4742a2b1-ca05-3e30-c2d2-ad3eada462fa@rymes.com> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6109423357768400083==" List-Id: --===============6109423357768400083== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 05/20/2019 12:42 PM, Bernhard Bitsch wrote: >=20 >=20 >> Gesendet: Montag, 20. Mai 2019 um 18:03 Uhr >> Von: "Tom Rymes" >> An: development(a)lists.ipfire.org >> Betreff: Re: Aw: Re: [PATCH] Fix for Bug #12050: Adding fixed leases with = one 'add' click >> >> On 05/20/2019 10:21 AM, Bernhard Bitsch wrote: >> >> [snip] >> >>>>>> If you plan to change any behaviour of the CGI file, that is a matter = open for discussion first and then work should start. >>>>>> >>>>> >>>>> When is this discussed? I made a suggestion of changes of behaviour; ye= t: >>>>> - Adding a new fixed lease adds this directly, without having to click = a second time. >>>> >>>> That is already the case. >>> >>> That's not true! Adding a new entry must retain the existing entries, wha= t isn't the case ( see postings in forum ). >> >> I must chime in here that clicking the button add a dynamic lease >> shouldn't require a second click, IMHO. Add the dynamic lease as fixed, >> then present the user the option to change things, such as name, >> address, etc. after it has already been added. >> >>>>> - Adding a dynamic lease to the fixed leases should work in two steps: = first the data from dynamic leases is copied to the edit fields, user can cha= nge and complete the definition and adds this by clicking "add". A check for = disjunction of sets of fixed and dynamic leases would be possible. >>>> >>>> If you add it from the DHCP leases list, the static lease is meant to be= added right away, but you can still edit it to give it a better name or so. = This what currently seems to be broken. >>>> >>> >>> Maybe this the intention of the actual implementation. The misfunction la= ys in the bug, indeed. >>> But my suggestion goes one step further. It isn't desirable to mix up sta= tic and dynamic leases. This is based on the mechanics, how dynamic leases ar= e found by dhcpd ( see man page ). IMHO, the problem with this is not shining= up immediately, but some times later ( with no modifications meantime ). A t= wo step process with check if the two sets are disjoint avoids this problem. >> >> [snip] >> >> While I would agree that keeping the dynamic and fixed leases physically >> distinct is a good goal, that is a separate change from the newly >> introduced bug, and should be handled separately. IPFire has allowed the >> user to add fixed leases that overlap with the dynamic address scope for >> as long as I have used the product, and it's not really a high-priority >> issue IMHO, so mixing it up with the fix for the new bug seems like a >> bad idea to me. >> >=20 > We can guarantee the disjointness only, if we don't allow a definition whic= h breaks this condition. > My suggestion just should help to come out of this situation. Actually, that's not technically accurate. If you read the bug I filed,=20 you can see that writing the conf file in the proper way would also=20 solve this issue. >> Here is the bug I opened on that subject a number of years back: >> https://bugzilla.ipfire.org/show_bug.cgi?id=3D10629 >> >> Tom >> >=20 > Therefore I added the Bug # to the subject. > Why can't we fixed this bugs together? They are located just in same code p= arts. You can, but what with all of the complaints about messy, ugly,=20 hard-to-maintain code, I figure that breaking things down into pieces=20 would result in a faster, cleaner fix for the initial problem (#12050),=20 which is a significant issue. The fix for #10629 can wait, as it is=20 rarely an issue in practice, and that way we will be less likely to=20 introduce new problems fixing 10629 when the true goal is to fix 12050. Tom --===============6109423357768400083==--