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

Delegation issues in HTTP::Request::Caching #239

Closed
headius opened this issue Jul 21, 2015 · 4 comments
Closed

Delegation issues in HTTP::Request::Caching #239

headius opened this issue Jul 21, 2015 · 4 comments
Milestone

Comments

@headius
Copy link

headius commented Jul 21, 2015

While investigating jruby/jruby#3151, a few issues with the way Caching does delegation came to light:

  • At least one place, in conditional_on_changes_to, an HTTP::Request object gets double-delegated. Both the self.class.new and .caching calls wrap the new HTTP::Request object in a delegating Caching instance.
  • The Caching class itself is created before HTTP::Request has completed loading, resulting in a DelegateClass that does most delegation through method_missing rather than through generated, direct delegation methods.

These issues are not strictly bugs, but they likely affect performance. In the case of jruby/jruby#3151, they also exposed an obscure bug in JRuby.

@tarcieri
Copy link
Member

I'm not really happy with the quality of the caching code. It's caused a lot of sporadic test failures. Having a dependency on rack isn't really something I like either.

Perhaps we could remove it and add it back when/if it's in a better state?

@zanker
Copy link
Contributor

zanker commented Jul 21, 2015

I agree with removing the code until something better is done.

@ixti
Copy link
Member

ixti commented Jul 21, 2015

👍 for wiping it out.

@tarcieri
Copy link
Member

Created #240 to revert caching

@ixti ixti modified the milestone: v0.9 Jul 22, 2015
tarcieri added a commit that referenced this issue Jul 22, 2015
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

No branches or pull requests

4 participants