Encoding in StreamHandler in lewis 1.3

[chris-d-gregory]

I've hit a problem when upgrading from version 1.2.2 to 1.3. I may be overlooking an obvious solution, but I've suggested one at the end for your thoughts.

In the _send_reply method of the StreamHandler class, the way that strings are converted to bytes has been changed from six.b(s) to s.encode(), which is reasonable. This has broken some old code that required sending 4-byte float values. I used to use something like this in my stream interface:

def my_command(self):
  value = 100.5
  return struct.pack('>f',value).decode('latin-1') 

which was then correctly encoded again before being sent to the client. Now it gets encoded as the system default (utf-8 on my machine) which doesn't give the expected result, and I can't decode directly to utf-8 in my_command as not all bytes are in range.

My quick fix was to force the encoding to be latin-1 in _send_reply but I think a better solution might be to allow a return_mapping function of bytes and then checking if the value is already encoded in _send_reply. If it is then it could be sent without encoding (after appending the encoded line terminator). If not then carry on as before. It gives the user a bit more control over the encoding. For example:

def _send_reply(self, reply):
    if reply is not None:
        self.log.debug("Sending reply %s", reply)
        if isinstance(reply, bytes):
            self.push(reply + self._target.out_terminator.encode())
        else:
            self.push((reply + self._target.out_terminator).encode())

I'd be grateful for any pointers.

[mattclarke]

Hi @chris-d-gregory. Thanks for getting in touch.

I like your solution of only encoding it not already encoded so if the user wants to choose a different encoding then it just works. Do you fancy putting together a PR?

[chris-d-gregory]

Yes, I'd be glad to. There is a slight problem with my solution that the the line terminator still gets encoded in the machine default, which might cause problems for more exotic terminators I suppose. I've never faced that though. I'll think about what to do about it and submit a draft PR.

[chris-d-gregory]

Turns out this is already implemented in the main branch - just not in release 1.3.0 - so there is nothing to do. We can just work with the latest commit to fix this so the issue can be closed.

[mattclarke]

Cool - thank you. I can make a new release if that helps?

[chris-d-gregory]

That would certainly help, thanks. If you could upload the new release to PyPI in addition to tagging on Github that would be fantastic, although I appreciate that's extra work.

[mattclarke]

Done - give it a try and shout if you hit any problems

[chris-d-gregory]

Thanks Matt. We're using it now without any problem so far.