add middleware to parse iso dates #40

Merged
merged 3 commits into from Jun 21, 2012

Conversation

Projects
None yet
3 participants
Contributor

grosser commented May 30, 2012

to be placed for instance after json response middleware

{"created_at" => "2012-02-02T12:13:15Z"} --> {"created_at" => Time-Object}

Contributor

mislav commented May 30, 2012

Interesting. I'll think about whether to add it. Maybe this should be a feature of JSON middleware?

Aren't there JSON parsers that deserialize timestamps, BTW?

Contributor

grosser commented May 30, 2012

Not sure if there already is a whole parser that deserializes timestamps.
I first implemented it as part of the json parser, but then realized that it could sit behind an xml/yml/xxx parser as well

Contributor

grosser commented May 30, 2012

this could also be generalized as parse_date middleware where you can pass in a regex and would default to iso format

Member

sferik commented May 30, 2012

I think it makes sense to have a separate middleware for date parsing but I do think the approach of using a single regex is too narrow. APIs return dates in all sorts of crazy formats. This middleware should be able to handle as many formats as possible.

Contributor

grosser commented May 30, 2012

json usually uses the iso format, but making the regex overrideable should
be fairly easy, Ill take a shot at it

On Wed, May 30, 2012 at 10:19 AM, Erik Michaels-Ober <
reply@reply.github.com

wrote:

I think it makes sense to have a separate middleware for date parsing but
I do think the approach of using a single regex is too narrow. APIs return
dates in all sorts of crazy formats.


Reply to this email directly or view it on GitHub:
https://github.com/pengwynn/faraday_middleware/pull/40#issuecomment-6015716

Contributor

grosser commented May 31, 2012

There you go :)

Contributor

mislav commented Jun 4, 2012

@grosser Looks good. Still not sure about including this middleware, but you did everything right. It's configurable, simple, has tests. I'll let this brew in my mind until the next feature release.

The only thing in tests that I don't like is that you pass JSON strings to process, but then parse them to Ruby hashes in your own overloaded version of process. Why the unnecessary JSON step? I would simply use plain ol' hashes to begin with.

Contributor

grosser commented Jun 4, 2012

hmm I wanted the tests to feel real, but you are right its unnecessary overhead :)

@ghost

ghost commented Jun 18, 2012

I just migrated some legacy code to use Faraday, and one of the problems was that old JSON parser was converting timestamp strings to objects, and new JSON parser from Faraday is not doing that. Having a plugin that can post process results of JSON parser and handle those timestamps would be really great, therefore huge +1 for feature in this pull request.

@sferik sferik added a commit that referenced this pull request Jun 21, 2012

@sferik sferik Merge pull request #40 from grosser/iso-dates
add middleware to parse iso dates
7cad490

@sferik sferik merged commit 7cad490 into lostisland:master Jun 21, 2012

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