search

Pomax / node-flickrapi

A node.js (and client-library) implementation of the Flickr API with oauth API key authentication and API method proxying
177 stars 52 forks source link

Adding ability to add default options for request. #92

Open ghost opened 7 years ago

ghost commented 7 years ago

Sometimes the flickr api takes a lot of time to respond with content. While using heroku for deployment, usually heroku times out a request every 30 seconds, so I would like to timeout the request to the flickr api sometime before the 30 second timeout is reached. Presently for request.get there is no way to add default parameter to request. It would be great to have one, so that I can configure the requests for the flickrapi as well.

ghost commented 7 years ago

sent a PR https://github.com/Pomax/node-flickrapi/pull/91

Pomax commented 7 years ago

Hm, in this case I'd not make this a two level object, but make that an explicit value in the options, like requestTimeout in the options, and then unpacking that value when giving request the options it needs.

Using request.defaults(options.requestOptions) means that rather than offering a way to do timeouts, we're offering a way to do everything that request.defaults lets you overwrite, in which case we should tell people where the documentation for that is, instead of claiming that there's only one value supported.

Either way, the PR will probably need a bit of a rewrite to either offer a dedicated value for just this specific use, or to handle the request options object as literally that; an object to pass into request.defaults

ghost commented 7 years ago

If another day someone wants to add another option to the request.defaults, like gzip: true, then (s)he would have to write add that explicitly to this library. So I think letting the user pass the defaults to the request.defaults as is will be better.

Pomax commented 7 years ago

agreed - if you can update the link to the request.default docs specifically I think we can just merge this in.