Feature request: Pass in dhcpd.leases as a string

[tdobes]

Feature request: It'd be nice if the code to read from a file could be separated from the code to parse the dhcpd.leases file. I'd like to be able to pass in the contents of the dhcpd.leases file as a string. (instead of reading from a file)

For my specific application, I'll be retrieving the contents of the file over the network, as I'm parsing it on a separate system. I'd like to be able to just pass in the file contents as a string instead of having to write a temporary file in my code then immediately re-open that file using this library.

Thanks!

[andir]

I think there was some discussion about this in the past but I can not recall where and what the result was.

For the time being a temporary file would probably be the simplest solution (for you).

Looking at the code again I think we have to break the current API in order to provide a proper interface.

@MartijnBraam what do you think about these versions?

leases = IscDhcpLeases.from_file("dhcpd.leases")
# and
leases = IscDhcpLeases('''
  lease 10.0.0.10 {
  starts 2 2013/12/10 12:57:04;
  ends 2 2013/12/10 13:07:04;
  tstp 2 2013/12/10 13:07:04;
  cltt 2 2013/12/10 12:57:04;
  binding state free;
  hardware ethernet 60:a4:4c:b5:6a:dd;
  uid "\377\000\000\000\002\000\001\000\001\0321\301\300\000#\213\360F\350";
  set vendor-class-identifier = "Some Vendor Identifier";
}
''')

Where the first version is pretty accepts a path to a filename. The second version accepts a string like object or an (open) file handle.

For example:

class IscDhcpLeases:
  @classmethod
  def from_file(cls, filename):
    with open(filename) as fh:
      return cls(fh)

  def __init__(self, fh_or_string, …):
    if isinstance(fh_or_string, io.TextIOBase):
      data = fh_or_string.read()
    else: # isinstance(fh_or_string, str)
      data = fh_or_string
    self._process_data(data)

  def get(self):
    return self._leases

I would be willing to implement that if we come up with a decision.

[MartijnBraam]

I think its a good idea to split the data processing and file handeling from the get() method.

If we change the __init__ to this signature then it is not a compatability changing change anymore:

class IscDhcpLeases:
    def __init__(self, filename=None, gzip=False, contents=None):
        if filename is None and contents is None:
            raise ValueError("You need to specify either `filename` or `contents`")
        if filename is not None and contents is not None:
            raise ValueError("You need to specify exactly one of  `filename` or `contents`")

        if contents is not None:
            self._process_data(contents)
        if filename is not None:
            # Process filename and gzip the old way

What do you think?

[tgandor]

I would rather go for 'smart' behavior, i.e.: def __init__(self, leases_file):, and then:

if leases_file is an "open stream", then process it directly; maybe detect b'\x1f\x1b' and then gunzip it on the fly.

if leases_file is a path (say, in terms of os.path.exists) - like currently, but autodetect gzip like above.

If you hate magic, a completely different method is to add static methods from_string and from_stream (or something like that, think json.loads and json.load).

Adding multiple arguments to constructor, and then processing them in arbitrary order, or giving one precedence over others - I would advise against ;)

[dholl]

To get the discussion moving again, I'll create a pull request since I have the same need for parsing from a string.