Frame building API #13

Closed
jonasschneider opened this Issue Jan 3, 2012 · 3 comments

Comments

Projects
None yet
2 participants
Collaborator

jonasschneider commented Jan 3, 2012

What came to my mind after thinking about the 'return parsed packets' pull:

It's a good idea to abstract the BinData records for parsing packets.
Maybe there should also be some kind of 'Framing API', to also abstract the internals for when creating frames... what do you think?

Owner

igrigorik commented Jan 4, 2012

I recall having the same idea when I was fiddling with implementing a spdy server way back.. Do you have any ideas for the API? If we can improve the interface.. could be interesting.

Collaborator

jonasschneider commented Jan 5, 2012

Maybe the simplest way would be to also hide the BinData structs, and return binary strings directly:

f = SPDY::Framer.new(@zlib)
f.syn_stream(stream: 1, associated_stream: 3) #=> "\x80\x02..."
f.headers(stream: 1, headers: {'a' => 'b'}) #=> "\x80\x02..."
f.data_frame(stream: 1, data: 'Hi there') #=> "\x00\x00..."

Then we are flexible with the implementation, and open to maybe rewriting it as a C extension someday, or whatever.

Owner

igrigorik commented Jan 7, 2012

I like the idea of abstracting away the BinData guts.. since as you pointed out, that decouples us down the road from swapping in a different implementation.

(I have nothing against bindata, but I chose it originally due to its simplicity to prototype with, over anything else)

Having said that, having an API which spits out the binary strings directly is also a bit inflexible - no? Just for sake of evaluating the alternatives:

SPDY::Protocol::Data::Frame.create( { ... } ) - we could keep the objects but create a single step function to emit the binary string. It's a bit more verbose with all the namespaces, but the upside is that it's very explicit.

For the frame api, if we were to go with that approach, I'd keep the .method on the framer as exact packet names, to remove any ambiguity: .synstream, .headers, .data, etc.

igrigorik closed this Oct 30, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment