New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support different json serialization types in /api/rows #210

Closed
maxogden opened this Issue Nov 4, 2014 · 4 comments

Comments

Projects
None yet
3 participants
@maxogden
Member

maxogden commented Nov 4, 2014

similar to how the /api/changes style option works

related IRC discussion:

5:31 PM <@ogd> karissa: so all we have in the dat rest api right now to expose leveldb.createReadStream is https://github.com/maxogden/dat/blob/master/lib/rest-handler.js#L233-L241
5:31 PM <@ogd> karissa: that is fine for basic stuff but it would be nice to be able to get different return forms
5:31 PM <@ogd> formats*
5:31 PM <karissa> yes
5:32 PM <@ogd> karissa: for the changes feed we use this module https://github.com/mikeal/SLEEP/blob/master/server.js#L40
5:32 PM <@ogd> karissa: but its kind of awkward cause only the changes feed supports that stuff
5:32 PM <@ogd> karissa: and it was added to dat really early and we never really talked about the API much
5:33 PM <@ogd> karissa: so its kind of weird cause when you do e.g. http://npm.dathub.org/api/changes?limit=5#
5:33 PM <@ogd> you get ndjson by default
5:33 PM <karissa> mm
5:33 PM <karissa> right
5:33 PM <@ogd> but with http://npm.dathub.org/api/rows?limit=5 its an array of objects
5:33 PM <@ogd> but you can do http://npm.dathub.org/api/changes?limit=5&style=object
5:34 PM <@ogd> karissa: and i dont even know if a querystrim param is the most restful thing to do here
5:34 PM <karissa> yeah we need a uniform way of doing this
5:34 PM <karissa> yeah querstring is right
5:34 PM <karissa> for that option
5:34 PM <@ogd> yea cause theyre all json so it wouldnt be accept: header i guess
5:34 PM <@ogd> karissa: so i was thinking there should be a thing that formats ndjson streams 
5:34 PM <@ogd> karissa: and just implements all the different formatters we use
5:34 PM <@ogd> karissa: so we can just re-use it everywhere we expose streams of json
5:35 PM <karissa> yup, that makes sense
5:35 PM <karissa> it can take the req and parse the options
5:35 PM <@ogd> karissa: or maybe even just take a pre-parsed options object, then its more pluggable
5:36 PM <@ogd> karissa: or have the http server convenience methods that take the req obj and parse it and then set the response type correctly and stuff
5:36 PM <@ogd> karissa: but also have the plain object formatting stuff available too

@maxogden maxogden assigned maxogden and karissa and unassigned maxogden Nov 4, 2014

@finnp

This comment has been minimized.

Show comment
Hide comment
@finnp

finnp Nov 10, 2014

Contributor

@karissa Did you start working on this? Otherwise I would try to implement it.

Contributor

finnp commented Nov 10, 2014

@karissa Did you start working on this? Otherwise I would try to implement it.

@karissa

This comment has been minimized.

Show comment
Hide comment
@karissa

karissa Nov 10, 2014

Collaborator

no go for it @finnp !

Collaborator

karissa commented Nov 10, 2014

no go for it @finnp !

@finnp

This comment has been minimized.

Show comment
Hide comment
@finnp

finnp Nov 11, 2014

Contributor

Okay here a few thoughts.

I think it would be nice to support accept headers. However they are often not very practicle, so I think it should be possible to overwrite them by specifying format in the querystring.

Here's what I would support:

format mime type (Accept headers/ Response headers)
csv text/csv
ndjson application/x-ndjson
json application/json
sse text/event-stream

We could then default this to json and keep the style parameter. This way it wouldn't break.

I wrote a small module (https://github.com/finnp/format-data) that can be used with an argument like {format: 'json', style: 'object' } and will return the fitting Transform stream.

On top of this I would start building a req/res handler, that parses the mime times and sets the response headers.

Contributor

finnp commented Nov 11, 2014

Okay here a few thoughts.

I think it would be nice to support accept headers. However they are often not very practicle, so I think it should be possible to overwrite them by specifying format in the querystring.

Here's what I would support:

format mime type (Accept headers/ Response headers)
csv text/csv
ndjson application/x-ndjson
json application/json
sse text/event-stream

We could then default this to json and keep the style parameter. This way it wouldn't break.

I wrote a small module (https://github.com/finnp/format-data) that can be used with an argument like {format: 'json', style: 'object' } and will return the fitting Transform stream.

On top of this I would start building a req/res handler, that parses the mime times and sets the response headers.

@finnp

This comment has been minimized.

Show comment
Hide comment
@finnp

finnp Dec 10, 2014

Contributor

@karissa This can be closed now 🎉

Contributor

finnp commented Dec 10, 2014

@karissa This can be closed now 🎉

@karissa karissa closed this Dec 19, 2014

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