quest
quest copied to clipboard
Release on hex.pm
Hi,
Do you think it would be a good idea to publish this package on hex.pm?
As far as I can see, it is fairly usable with reasonable defaults. The only problem would be the hard dependency to Ecto. I think it should be made optional like httpoison.
Hi, @lud thanks for your interest!
I still have mixed feelings, as in the README, about whether the value/effort to make a hex package is worthwhile. The other open issue (#2) points out some shortcomings in the code as it is.
The Eco dependency is because I added the AntiCorruptionSchema in here, again mostly as an example. I use it in conjunction with Quest, but they really wouldn't belong in the same hex package, so that would be easy to remove.
I'm open minded to polishing this up to be worth publishing on hex. This is probably not a complete list, but here's some of the things I'd want to get done first
- remove AntiCorruptionSchema and Ecto dep
- handle all 20x response codes to be an :ok tuple
- consider returning the original Quest struct as 3rd element of tuple from Dispatcher.dispatch
- more test coverage
- I'd like to at least introduce the optional Gateway concept a bit more, so I at least document good ways of extending/customizing logic
My free time is limited and once I get past the things that are just in my head, perhaps with some assistance, this could turn into enough code to be worth publishing as a library. What are your thoughts?
I had some very similar code of my own, defining a "client" (which is actually just a bunch of default headers, a base path, etc.) and then augmenting it to perform actual requests, all through an opaque-ish API. I was glad to see someone did the same, and in a more polished manner.
A current side project of mine happens to uses the gitlab API to fetch issues with this exact pattern. And those result are paged. I will try to see what I can do based on your code. I hope I can tackle this next week.
For the Gateway, throttling involves either processes or blocking the current process. I am not sure I want to tackle this at this level in the code. Generally I use a tool that I made (unfortunately undocumented) that I calll Drip and is a global throttler: I has a "number per seconds" allowed calls and is a named process, for instance some API for addresses in France allows maximum 10 calls in a second so all the calls to their API in my app passes through the Drip and respect that.
For logging and other non-time-related affairs, a gateway could be a mere middleware/decoractor/wrapper around the dispatcher option. In that way, using a drip or other blocking throttle mechanisms could be abstracted.
I don't feel a library should only return the body of the response unless I ask for it explicitely. Sometimes the headers are worth looking into. Our APIs also return JSON bodies with errors, to tell what field is missing or which key/value was incorrect. Quest abstracts the request and not the response. I have written a body-to-json helpers countless times, just as in Quest, but for a library I would expect an explicit json/json! function, and a generic parse/parse! that will try json if the content-type is set appropriately. Although I am not sure that an abstract Quest.Response is worth it. Those helper could just be a protocol for HttPoison.Response and equivalents. Well, Quest could be a protocol too actually.
The pager is a different beast. I could define a simple function fetch_all(quest, callback)
and the callback would receive the response, and return either {:cont, items_or_stream, next_quest}
where next_quest
is a new quest with the new page number or next cursor, or {:halt, item_or_stream}
for the last page. I know it would be cool to provide just an url param for the page number but that would also require to pass a callback just to read if there is a next page, and another callback to extract the items to stream (as they can be nested in the json result).
Sorry I just pull thoughts out my brain, my message is not very clear :)
You message was clear enough. I understand what you're saying.
Yes, my Gateway is called from the http_req
function of the service module. The Gateway does what it needs to do and then calls Quest.dispatch
. Much like middleware, but just one layer that you own so it matches your app's needs.
I use Stream.unfold
for my unpager. It has an accumulator that knows whether to return an item that was already fetched, or to fetch a new page or when we're reached the end.
You make a good case for a dispatcher that returns a richer data structure. The example here has served my needs, but to publish this as a hex package I would need to anticipate the needs of other apps. I will continue to think about polishing this into a published library, but I don't have much time to spend on it now. I will leave it as a pattern/example until then.
Hi,
Thank you for the input!
I will leave it as a pattern/example until then.
Alright, I will try to apply the pattern to my own code to see where it leads me.
I just watched the talk mentioned in the readme file, and while it was very clear, there is a strong incentive to diverge from the traditional ok/error tuples and use either raise or a halter/errored flag on the token for the whole api. That would be the first thing to make generic.
What would you choose to carry the response data/headers/status: Fields in the Quest struct or a new struct type that would be returned by dispatch, with its own api (parse_json,get_header,…) ? In the second case, maybe dispatch
would be the single function where an ok/error tuple would still be returned, to make it clear that we are swtiching the token type.
I wasn't particularly trying to follow Rene Föhring's approach. I mentioned his talk because he used the term "token" for this pattern. I personally like "accumulation pattern" more, but I recognize it may not be the best name.
I haven't thought a lot about a richer response struct. It sounds like a lot of boring code just wrapping around whatever response type the underlying http client library uses just for the sake of abstraction. That's partially why I haven't done that yet. I would probably lean toward a new struct and module for the response with its own API as you suggest. I'd include a field in the response struct for the original Quest request for debugging convenience.
To me it would rather be implementing common patterns like decoding json, expecting specific status code, cachcing a fetched stream, etc but I agree with you, it may not be an abstraction layer that is needed.