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

Limit method_missing to only handle known commands #73

Merged
merged 7 commits into from
Dec 5, 2013
Merged

Limit method_missing to only handle known commands #73

merged 7 commits into from
Dec 5, 2013

Conversation

yaauie
Copy link
Member

@yaauie yaauie commented Dec 3, 2013

Redis-Namespace has had a long history of issues rising from the blind
passing of unknown commands to the underlying Redis Client (see almost
every single issue in the tracker). This patch is a breaking change
that stops that behaviour, instead raising a NoMethodError when handling
a command that is unknown.

TODO before merge and 2.x release:

  • Add support & specs for Command Aliases Redis-RB hasn't supported these since 2.0, we require 3.0 as a minimum.
  • Define all methods to avoid method_missing altogether
    (via Module#define_method?)

Pairing session with @chelseakomlo and @yaauie.

chelseakomlo and others added 7 commits December 4, 2013 06:54
Redis-Namespace has had a *long* history of issues rising from the blind
passing of unknown commands to the underlying Redis Client (see almost
every single issue in the [tracker][]). This patch is a breaking change
that stops that behaviour, instead raising a NoMethodError when handling
a command that is unknown.

TODO before 2.x release:
 - [ ] Add support & specs for Command Aliases
 - [ ] Define all methods to avoid method_missing altogether
       (via `Module#define_method`?)

[tracker]: https://github.com/resque/redis-namespace/issues?state=closed

Pairing session with @chelseakomlo and @yaauie.
Support for command aliases was dropped in redis-rb 2.0, and we have
been requiring redis-rb 3.x for a while now, so the referenced code
is orphaned and *never* used.
@yaauie yaauie mentioned this pull request Dec 5, 2013
18 tasks
yaauie added a commit that referenced this pull request Dec 5, 2013
Only wrap known commands, eliminating the root cause of the
vast majority of past issues in this project.
@yaauie yaauie merged commit 9e9315a into master Dec 5, 2013
@yaauie yaauie deleted the 2.0 branch December 5, 2013 08:51
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.

None yet

2 participants