Skip to content

Conversation

@rstms
Copy link

@rstms rstms commented Apr 24, 2019

I made these changes because I needed this to run on my OpenBSD 6.4 system, which uses a dhcpd that is derived from isc-dhcpd. The dhcpd.leases file has minor differences which required changes to the parser. The existing tests all still pass.

These changes allow the module to read the OpenBSD lease file:

ignore 'UTC' suffix on time fields
record with missing binding_state defaults to 'active'
interpret 'abandoned;' field as 'binding_state abandoned;"
added test case for openbsd dhcpd.leases
added Makefile for make test

ignore 'UTC' suffix on time fields
record with missing binding_state defaults to 'active'
interpret 'abandoned;' field as 'binding_state abandoned;"
added test case for openbsd dhcpd.leases
added Makefile for `make test`
@coveralls
Copy link

Coverage Status

Coverage decreased (-23.0%) to 77.039% when pulling 7fc81df on rstms:master into e96c00e on MartijnBraam:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-23.0%) to 77.039% when pulling 7fc81df on rstms:master into e96c00e on MartijnBraam:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-23.0%) to 77.039% when pulling 7fc81df on rstms:master into e96c00e on MartijnBraam:master.

@MartijnBraam
Copy link
Owner

Thanks for the PR!, the code looks good but the unit test is broken because you've added a method to the test case with the same name as another method, so most tests don't run currently.

self.binding_state = 'active'

# support 'abandoned' property for openbsd dhcpd.leases compatibility
if 'abandonded' in properties:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/abandonded/abandoned/ ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you.

@raspbeguy
Copy link

Hello,
Sorry for necrobumping this PR, but I really wish it to be merged.
@rstms Is this a problem to fix the unit tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants