-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
QueueAdapters
now does lazy lookup
#19311
Conversation
mutex.synchronize do | ||
return instances.fetch(hash) if instances.key?(hash) | ||
|
||
@instances[hash] ||= lookup(name).new(*adapter_args, &block) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does we need to cache?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so - in particular, we may have an adapter that carries a connection object on it - we would not want to create multiple identical connections
Correct me, if I am mistaken: isn't Also, does setting Is there a reason you're not using |
Probably true - just erring on the side of caution.
Again, probably true - just erring on the side of caution.
Just plain ignorance. I'll switch to that. |
|
||
private | ||
|
||
attr_accessor :cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use a @cache
ivar, no need for private reader/writer methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
7a165ca
to
9832e17
Compare
Removed the caching - I'll save it for a future PR. |
`QueueAdapters` now does lazy lookup
@rafaelfranca @jeremy