Redis npm

[msimerson]

Changes proposed in this pull request:

• replace plugins/redis with haraka-plugin-redis
  • haraka-plugin-redis must exist for npm packaged plugins to test outside of Haraka (karma, known-senders, limit, watch)
  • additional testing was added in the new version
• while we should be able to drop the redis dependency (since haraka-plugin-redis has it as a dep), we can't do that yet because of the vm tomfoolery.

asides

• this removes the redis plugin tests from this repo
• if the last couple redis-consuming plugins get repackaged to npm (along with their tests), then the haraka test suite can once again pass without a redis server available.

Checklist:

• docs updated
• tests updated

[codecov-io]

Current coverage is 36.11% (diff: 100%)

Merging #1786 into master will not change coverage

@@             master      #1786   diff @@
==========================================
  Files            22         22          
  Lines          5795       5795          
  Methods         750        750          
  Messages          0          0          
  Branches       1454       1454          
==========================================
  Hits           2093       2093          
  Misses         3702       3702          
  Partials          0          0          

Powered by Codecov. Last update d5e98ee...b739231

[baudehlo]

while I should be able to drop the So much suspense and mystery in this PR.

…

On Jan 27, 2017, at 9:26 PM, Codecov ***@***.***> wrote: Current coverage is 36.11% (diff: 100%) Merging #1786 into master will not change coverage @@ master #1786 diff @@ ========================================== Files 22 22 Lines 5795 5795 Methods 750 750 Messages 0 0 Branches 1454 1454 ========================================== Hits 2093 2093 Misses 3702 3702 Partials 0 0 Powered by Codecov. Last update d5e98ee...b739231 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[msimerson]

So much suspense and mystery in this PR.

Did the message you received get truncated? Maybe visit the PR for the rest of the description?

[baudehlo]

Yeah weirdly this is what Apple mail on iOS shows:

…

On Jan 28, 2017, at 2:49 AM, Matt Simerson ***@***.***> wrote: So much suspense and mystery in this PR. Did the message you received get truncated? Maybe visit the PR for the rest of the description? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[bmatson]

Looks like you missed the required changes in the rate_limit.js plugin...the version currently in master looks like it's broke based on this change since it's still using the redis plugin.

[msimerson]

Aye, 'tis true. Since you're running out of master, might you care to try the new limit plugin (see also #1785) with rate limit support?

[bmatson]

Unfortunately I'm not actually running out of master, just the last release with a slightly hacked up local copy of rate_limit.js to get around the race condition I wrote up. I did give the PR limit plugin a code review and I see where it does not seem susceptible to the same type of race conditions with redis..... I'm keeping track of the changes just to make sure the "next" release has functional rate limiting, whether from the "old" rate_limit.js or your new limit plugin doesn't make much difference to me.

[msimerson]

In case you don't already know, the right way to use your local hacked rate_limit is by copying it to /path/to/haraka/config/plugins, and Haraka will prefer that to the installed version in the Haraka install dir.

Same goes for npm packaged plugins. If you:

cd /path/to/haraka
npm install haraka-plugin-limit

and then add limit to your config/plugins file, the 'local' one in your config directory takes precedence.