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

Use ioredis as Redis client #372

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

megawebmaster
Copy link
Contributor

Hi! 👋

I was in need of ability to use Redis Sentinel, so after reading #271 I decided to go with a simple PR that will update current node-redis package with ioredis package that has the same API, but supports more options and is fully featured to work with new Redis.

I also bumped package version to 3.0.0 due to possible breakage although I noticed that both packages have the same options supported. I'd like to make sure that users of this package will take a look and make sure that their app still works.

My changes pass all tests with my other PR #371 but I want to show you the only changes I made (I tested it locally).

Hope this helps folks over the world using Meteor 💪

@fantostisch
Copy link
Contributor

Hi, after merging #371 the tests still fail.

rm -rf test
meteor create --release 1.8.1 --bare test
cd test
meteor npm i --save [email protected] simpl-schema
METEOR_PACKAGE_DIRS="../" TEST_BROWSER_DRIVER=puppeteer meteor test-packages --raw-logs --once --driver-package meteortesting:mocha ../
[[[[[ Tests ]]]]]                             

=> Started proxy.                             
=> Meteor 2.3.4 is available. Update this project with 'meteor update'.
=> Started MongoDB.                           
Note: you are using a pure-JavaScript implementation of bcrypt.
While this implementation will work correctly, it is known to be
approximately three times slower than the native implementation.
In order to use the native implementation instead, run

  meteor npm install --save bcrypt

in the root directory of your application.
/home/nick/.meteor/packages/meteor-tool/.1.8.1.14dd419.u5ho++os.linux.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.linux.x86_64/dev_bundle/server-lib/node_modules/fibers/future.js:280
						throw(ex);
						^

Error: Cannot find module 'chai'
    at Function.Module._resolveFilename (module.js:548:15)
    at Function.resolve (internal/module.js:18:19)
    at Object.require (/tmp/meteor-test-run11u5fhw.qhfig/.meteor/local/build/programs/server/boot.js:296:32)
    at makeInstallerOptions.fallback (packages/modules-runtime.js:618:18)
    at Module.require (packages/modules-runtime.js:244:14)
    at Module.moduleLink [as link] (/home/nick/.meteor/packages/modules/.0.13.0.pm7my8.14b0j++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/reify/lib/runtime/index.js:38:38)
    at getFields.test.js (packages/local-test:cultofcoders:redis-oplog/lib/utils/testing/getFields.test.js:1:19)
    at fileEvaluate (packages/modules-runtime.js:336:7)
    at Module.require (packages/modules-runtime.js:238:14)
    at Module.moduleLink [as link] (/home/nick/.meteor/packages/modules/.0.13.0.pm7my8.14b0j++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/reify/lib/runtime/index.js:38:38)
    at index.js (packages/local-test:cultofcoders:redis-oplog/lib/utils/testing/index.js:1:8)
    at fileEvaluate (packages/modules-runtime.js:336:7)
    at Module.require (packages/modules-runtime.js:238:14)
    at Module.moduleLink [as link] (/home/nick/.meteor/packages/modules/.0.13.0.pm7my8.14b0j++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/reify/lib/runtime/index.js:38:38)
    at main.server.js (packages/local-test:cultofcoders:redis-oplog/testing/main.server.js:1:80)
    at fileEvaluate (packages/modules-runtime.js:336:7)
=> Started your app.

=> App running at: http://localhost:3000/

@megawebmaster
Copy link
Contributor Author

megawebmaster commented Aug 11, 2021

@fantostisch You should run it a bit differently (based on .travis.yml file):

meteor create --release 1.8.1 --bare test
cd test
meteor npm i --save [email protected] simpl-schema chai
METEOR_PACKAGE_DIRS="../" TEST_BROWSER_DRIVER=puppeteer meteor test-packages --raw-logs --once --driver-package meteortesting:mocha ../

As you can see chai is added in meteor npm i call.

@fantostisch
Copy link
Contributor

Thanks @megawebmaster, it works now. I think the documentation should be updated https://github.com/cult-of-coders/redis-oplog/blob/master/CONTRIBUTING.md#setup

@megawebmaster
Copy link
Contributor Author

@fantostisch - I added it to this PR too 👍 Good find!

@megawebmaster
Copy link
Contributor Author

@theodorDiaconu - did you have time to take a look at my PR? I find it sad that nobody from the team cared enough to tell me if you're OK with the changes or not 😢

@maxpain
Copy link
Contributor

maxpain commented Apr 8, 2022

Any news?

@maxpain
Copy link
Contributor

maxpain commented Apr 8, 2022

I also need this PR to be merged :)

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.

4 participants