RFC: deprecate write_bytes option in unpackers

[methane]

I feel write_bytes option in Unpacker makes implementation complicated.
I forgot why I added it. I'll check history and consider another API for the usecase.

Ideas? comments?

[methane]

msgpack-python has too many options other msgpack implementation doesn't have.
I doubt some people uses this library for usecases which is not target of msgpack.

[jfolz]

This? #23

Seems like a convoluted way to do framing.

[methane]

cc: @jnothman

At least reading #23, target is not "avoid full unpacking", but "avoid repacking".
Since skip() and read_map_header() can be used to avoid full unpacking.

And I think write_bytes is not optimal way to achieve the target.

[methane]

#198 makes clear what write_bytes does.
https://github.com/msgpack/msgpack-python/pull/198/files#diff-0924a5fc6aaa4b057364baf18aa2c267R546

In Packer, we have similar option autoreset. But I feel autoreset is not good, too.
I'll look more later when I have time.

[methane]

Current API is bad because write_bytes is just a additional return value semantically.

Current API:

buff = io.BytesIO()
obj = up.unpack(write_bytes=buff.write)
raw = buff.getstring()

is same to:

obj, raw = up.unpack_and_capture()  # Is "capture" best name?

I dislike callback only for return value. There are some options for new API.

1. Add capture option

obj, raw = up.unpack(capture=True)

Pros: simple
Cons: Cant add typehint to .unpack() method.

2. Add .unpack_and_capture() method

obj, raw = up.unpack_and_capture()

Pros: clean
Cons: What should I do when adding more options? .unpack_and_capture_and_XXX() is horrible.

3. Add .start_capture() and .stop_capture()

up.start_capture()
obj = up.unpack()
raw = up.stop_capture()

Pros: User can capture serialized data for multiple .unpack() call efficiently.
Cons: Make unpacker more stateful

---

Since recent Python has type hinting, I dislike option 1. I'll choose from option 2 or 3.

I don't know is there more better name than capture, since my English vocabulary is too bad.

Any ideas?

[jfolz]

I'm not entirely convinced something like this is necessary at all.

Assuming it is, I would rather it be realized through a more useful API that allows access to the internal buffer of the Unpacker.

1. Tell unpack not to consume used buffer space. You can now unpack as many messages as necessary. This may also be useful when unpacking a large number of tiny messages.
2. Get a memoryview of the used buffer. This method can also be used to give access to the available space so it can be used for socket.read_into or similar.
3. Tell the Unpacker to free the buffer when ready.

[methane]

I'm not entirely convinced something like this is necessary at all.

Yes. Removing the API should be first option, unless ruby's msgpack implementation doesn't have same API. (Since ruby's msgpack is maintained the author of msgpack.)
http://ruby.msgpack.org/MessagePack/Unpacker.html

[jfolz]

Since @jnothman has not responded yet, can we assume this feature can be dropped for 1.0?
I can see use cases where one might want to read the next packed message from a raw data stream, e.g., when reading messages from a file. This operation is semantically similar to skip (as it looks at only parts of the data until it finds the end of the message) but would return the skipped data. That data can then still be unpacked if necessary.
Since skip has no return value right now, maybe it could do this?

[jfolz]

Might I also add that the current write_bytes implementation causes a lot of Python method calls to write tiny pieces of data one-by-one, whereas skip translates to nicely optimized C-code?
Removing functionality should always be the last option. It breaks code. However, semi-broken APIs have to be removed at some point. IMO nobody should use msgpack like this.

[jnothman]

Hi, I didn't see the ping in May, sorry. I think this was a context where the length of or exact message encoded was used, as well as the decoded object. I suspect getting the raw msgpack on skip would be a sufficient alternative. I agree that it is not a feature worth slowing down normal use, and I suspect you're going to break little code out there. (The case where I had used it it seems to be no longer used.)

[jfolz]

I don't think it's slowing down normal use (this should be easy pickings for branch prediction).
It's more about #191, i.e., if 1.0 is going to break code we might as well clean up the API in other places.

[methane]

I found relating question on Stackoverflow
http://stackoverflow.com/a/38726237/3430118

As I answered there, double-packing is idiom used by msgpack owner.

[acorver]

I just stumbled across this thread and wanted to make sure you're aware of the following:

write_bytes was previously suggested as a solution for indexing msgpack files (see here). If write_bytes becomes deprecated, what is the recommended approach for finding byte offsets of msgpack items within larger msgpack files?

Has write_bytes deprecation already been finalized?

[methane]

Other major language's msgpack libraries support it?
If no, why only Python should support it?

I want to know realistic and typical use case.
I don't want to make msgpack-python complicated to support niche use cases.

[jfolz]

IMO this is a classic case of "you're doing it wrong". messagepack is not a wire protocol or container format. Sending raw JSON messages over a socket or dumping them in a file one after the other is almost always a terrible idea. If you need more functionality than just reading one message after the other you need some form of protocol around it.
If you can't fix your protocol and desperately need to index a packed stream, I suggest using read_size instead. Look at this gist:
https://gist.github.com/jfolz/74b130649e9134c35b5fcf200a1ddbe7
It tells the Unpacker to read from the stream one byte at a time. Sounds very slow, so we wrap that stream in a BufferedReader.
Timings (py3.5):

write_bytes 1.3209055614557093
read_size 1.2869876740423465

It's too close to really call one implementation faster than the other, but read_size needs just one extra line for the BufferedReader vs a callback and try-except block for write_bytes.