Skip to content
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

Extract rejection reason in Proxy instead of CacheHandler #541

Merged
merged 2 commits into from
Jan 5, 2018

Conversation

davidor
Copy link
Contributor

@davidor davidor commented Jan 5, 2018

Extracting the rejection reason and checking if the request has been authorized has nothing to do with the cache. We only need to check the response status and a header. This is why I'd rather not have this in the cache_handler module.

Apart from that, this change will simplify the implementation of the caching policy.

@davidor davidor requested a review from mikz January 5, 2018 11:23
@davidor davidor force-pushed the resp-and-reject-reason-in-proxy branch from 19403f4 to 729b9b0 Compare January 5, 2018 11:29
…che_handler

Extracting the rejection reason and checking if the request has been
authorized has nothing to do with the cache. We only need to check the
response status and a header. This is why I'd rather not have this in
the cache_handler module.

Apart from that, this change will simplify the implementation of the
caching policy.
Also stop checking whether the cache_handler methods return true or
false, they no longer return that.
@davidor davidor force-pushed the resp-and-reject-reason-in-proxy branch from 729b9b0 to 817c2d1 Compare January 5, 2018 11:31
@davidor
Copy link
Contributor Author

davidor commented Jan 5, 2018

@mikz I added a changelog and fixed a minor codeclimate issue.

Copy link
Contributor

@mikz mikz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice cleanup

@davidor davidor merged commit 25009f6 into master Jan 5, 2018
@davidor davidor deleted the resp-and-reject-reason-in-proxy branch January 5, 2018 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants