Added a test for MemoryRecordsBuilder and fixed some problems there #1448
Conversation
|
|
||
| __all__ = ["MemoryRecords"] | ||
| __all__ = ["MemoryRecords", "MemoryRecordsBuilder"] |
There was a problem hiding this comment.
I'm not very familiar with this code, but I don't understand why this import is needed? Reading the bug description, it looks like the problems were all due to the code fixed in kafka/record/memory_records.py below... If the import were just forgotten, I would have thought it would have thrown problems before?
There was a problem hiding this comment.
No particular reason, just used in tests. We can remove it, yea
There was a problem hiding this comment.
I'm fine leaving it in if you think it should be there, I just didn't understand if there was something I was missing.
| @@ -166,7 +166,7 @@ def size_in_bytes(self): | |||
|
|
|||
| def compression_rate(self): | |||
| assert self._closed | |||
| return self.size_in_bytes() / self._bytes_written | |||
| return self.size_in_bytes() / float(self._bytes_written) | |||
There was a problem hiding this comment.
This is only required for python2; python3 division returns a float by default. I think a cleaner way to handle this compatibility is with __future__:
from __future__ import division
With that you shouldn't need to cast to float
| @@ -135,7 +135,7 @@ def append(self, timestamp, key, value, headers=[]): | |||
| (int, int): checksum and bytes written | |||
There was a problem hiding this comment.
Return type in docstring is incorrect.
|
Merged manually |
Fixes #1442