Skip to content

Move string parsing to separate function#35

Open
dholl wants to merge 1 commit intoMartijnBraam:masterfrom
dholl:master
Open

Move string parsing to separate function#35
dholl wants to merge 1 commit intoMartijnBraam:masterfrom
dholl:master

Conversation

@dholl
Copy link
Copy Markdown

@dholl dholl commented Feb 27, 2021

This pull request addresses issue #24 by moving the string parsing code into a separate function. It preserves existing API usage, but adds a new publicly-exposed function.

@dholl
Copy link
Copy Markdown
Author

dholl commented Feb 27, 2021

Comments / changes are welcome! I'm indifferent to exactly how #24 gets implemented, so in this first draft, I attempted the minimum possible to isolate the parsing logic. But maybe folks want this as a static function instead?

Personally, instead of returning a list of leases, I'd rather tweak this function into returning an iterable to permit parsing very large lease files and filtering for active leases on-the-fly. If folks are okay with this idea, I'm happy to tweak this new function and update the pull request. It would change the "simple case" calling from

leases = isc_leases_from_string(the_string)

to

leases = list(isc_leases_from_string(the_string))

It would also allow for things like:

active_leases = list(filter(lambda lease: lease.valid and lease.active, isc_leases_from_string(the_string)))

or fancier:

for active_lease in filter(lambda lease: lease.valid and lease.active, isc_leases_from_string(the_string)):
    # do something like launch coroutine to investigate this lease...

but the basic idea is that for very large lease files, we never have to have an entire list of Lease objects in memory at once if we don't need it, especially since the contents of the lease file is still loaded as a string.

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.

1 participant