fixes #1172 - proxy should return leases and reservations #437
Conversation
| logger.debug opts.inspect | ||
| if opts.include? :hostname | ||
| to_return << Proxy::DHCP::Reservation.new(opts.merge(:deleteable => true)) | ||
| if kind == 'D' && expire !~ /^INACTIVE|^NEVER/ |
There was a problem hiding this comment.
Are you sure about kind == 'D' bit? I'm seeing 'N' for clients that support both bootp and dhcp? I think this flag is an "or" of various options and netsh isn't very good at parsing those...
My understanding is that we can treat any client without expiration date on its address as a "reserved ip".
There was a problem hiding this comment.
You may well be right on this point. This logic works for my organisation but this may not be generally true.
I will update the push to remove the "D" clause as just using the INACTIVE|NEVER test would return the same host list.
|
[test] |
|
If I wish to resubmit this pull request without the "D" then what is the right procedure? Do I close this PR and open another? Do I push another commit onto this branch? I expect that creating a new commit with a commit amend and then doing a forced push will break the PR as the commit's hash will change. |
|
@pikelly: you can make the change, squash the commits and force-push the changeset (what I'd do, as it's fastest). Or you can close this PR and open a new one, up to you really. |
|
IMHO better to push, as you dont need to spam redmine with the new pr number On Tue, Jun 21, 2016 at 5:18 PM, Dmitri Dolguikh notifications@github.com
|
pikelly
force-pushed
the
feature/1172-return-leases-and-reservations
branch
from
3770c44
to
5970b22
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
I have force pushed the commit minus the "D" check. |
This is my second attempt to get this code included. The last attempt was some time ago :-) You may want to read the earlier pull request or just judge it on its own merits.
This modification has been working in my organisation for some time and seems to separate out the leases and reservations correctly.