Fixed using Caching middleware with async adapter like Typhoeus #121
Conversation
All checks for response properties need to be done inside on_complete And need to actually return the response.
| if CACHEABLE_STATUS_CODES.include?(response.status) | ||
| response.on_complete { cache.write(key, response) } | ||
| # response.status is nil at this point, any checks need to be done inside on_complete block | ||
| response.on_complete do |
There was a problem hiding this comment.
I would prefer you to use
@app.call(env).on_complete do |response|instead of the longer
response = @app.call(env)
response.on_complete doI also think is better to use the block parameter rather than the scope response
|
Thanks @kmamykin and sorry for the late reply. I would simply ask you to fix what I highlighted in my review and pull latest changes from master to see if tests are green. Thanks! |
|
@iMacTia I made the change, and was very happy for a welcome disruption from my monotonous job. Thanks you. However rebasing and making the tests pass, I prefer to leave that in your capable hands, since you are already deep into it. |
|
Good news is that after the changes all tests are now passing 👍! |
* Fixed using Caching middleware with async adapter like Typhoeus
All checks for response properties need to be done inside on_complete
And need to actually return the response.
Related to the lack of tests for parallel mode: See #100