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

options.redis.opts.createClient no longer works? #711

Closed
ianstormtaylor opened this issue Oct 5, 2017 · 4 comments
Closed

options.redis.opts.createClient no longer works? #711

ianstormtaylor opened this issue Oct 5, 2017 · 4 comments

Comments

@ianstormtaylor
Copy link
Contributor

I've been trying to track down tests failing in super weird ways for many hours now, and it seems like in 3.x the options.redis.opts.createClient function is no longer called? I used to have it setup like this:

const client = new Redis(REDIS_URL)
const subscriber = new Redis(REDIS_URL)

function createQueue(name) {
  const queue = new Queue(name, {
    redis: {
      opts: {
        createClient(type) {
          switch (type) {
            case 'client': return client
            case 'subscriber': return subscriber
            default: return new Redis(REDIS_URL)
          }
        }
      }
    }
  })
}

Which is still how it's recommended in the PATTERNS.md document. And the MIGRATION.md document doesn't really mention what has changed.

Is there another way to do this now that's not documented?

@ianstormtaylor
Copy link
Contributor Author

For anyone who stumbles on this, apparently createClient has been moved to a top-level option:

const client = new Redis(REDIS_URL)
const subscriber = new Redis(REDIS_URL)

function createQueue(name) {
  const queue = new Queue(name, {
    createClient(type) {
      switch (type) {
        case 'client': return client
        case 'subscriber': return subscriber
        default: return new Redis(REDIS_URL)
      }
    }
  })
}

@ritter
Copy link

ritter commented Oct 5, 2017

For what it's worth ... you could update the documentation and submit a PR. I'm sure contributions like that are quite welcomed.

Was also mentioned in #545

@manast
Copy link
Member

manast commented Oct 5, 2017

I have updated it now.

@manast manast closed this as completed Oct 5, 2017
@ianstormtaylor
Copy link
Contributor Author

@manast thank you! I think in the future, the most helpful thing for me as a consumer of this library would be if the CHANGELOG.md was a bit more descriptive, and ensured to contain every possible breaking change in a very obvious way. Ideally they'd be bolded or something else to quickly scan. (Even if that meant not mentioning the new additions, since those are lower priority if there's not enough time.)

@ritter thanks, I definitely realize that—here's a past PR I made to the docs. In this case though I think it's pretty hard to ask people to contribute docs for undocumented breaking API changes they don't even know about when they go to upgrade.

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

3 participants