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

Issue with Redis while Upgrading or Installing Sentry #2530

Closed
joaoribeirost opened this issue Jan 11, 2016 · 7 comments · Fixed by #2714
Closed

Issue with Redis while Upgrading or Installing Sentry #2530

joaoribeirost opened this issue Jan 11, 2016 · 7 comments · Fixed by #2714
Assignees

Comments

@joaoribeirost
Copy link

Hello there,

While trying to upgrade to the most recent version of sentry , and following the documentation present at
this link i get the following error:

TypeError: add_host() got an unexpected keyword argument 'timeout'

I tracked down the problem to this part of the configuration file

SENTRY_REDIS_OPTIONS = {
    'hosts': {
        0: {
            'host': '127.0.0.1',
            'port': 6379,
            'timeout': 3,
            #'password': 'redis auth password'
        }
    }
}

What solved it for me was removing the timeout options. If this is the only solution maybe the documentation should be updated.

If anyone knows about another way of solving this share it. :)

I just wanted to share this.

@mitsuhiko
Copy link
Member

Thanks for the report. Looking into supporting this in rb.

@mitsuhiko
Copy link
Member

On a second thought: not sure how that made sense before. Pretty sure we never used the blocking connection pool.

@mattrobenolt do you know what the timeout there is supposed to be for? I can only see this used for the blocking pool as waiting time until connection is available and we are not using that (and did not before unless i am missing a change).

@mattrobenolt
Copy link
Contributor

@mitsuhiko This was the socket_timeout that used to get passed along to StrictRedis. See: https://github.com/disqus/nydus/blob/master/nydus/db/backends/redis.py#L67

@mattrobenolt
Copy link
Contributor

@mitsuhiko Not sure what needs to be done here, if anything?

@mitsuhiko
Copy link
Member

@mattrobenolt probably we should document that this is now called socket_timeout. I will check that it's supported in rb and we can put it into the docs. I rather not diverge from redis options going forward.

@mitsuhiko
Copy link
Member

@mattrobenolt actually kinda shitty. In rb this is configured with the pool options which you currently cannot set from sentry :(

@mitsuhiko
Copy link
Member

This issue will tackle supporting it under a new key: #2671

@mitsuhiko mitsuhiko assigned tkaemming and unassigned mitsuhiko Feb 16, 2016
tkaemming added a commit that referenced this issue Feb 20, 2016
This replaces `SENTRY_REDIS_OPTIONS` and per-backend configuration with
named Redis clusters, configured from the value of the `redis.clusters`
key in `sentry.options`. The preferred method of configuring Redis-based
backends is by providing a *cluster name*, rather than the cluster
configuration parameters.

This deprecates:

- `SENTRY_REDIS_OPTIONS` in favor of the `redis.clusters` option.
- Passing `hosts` to any backend in favor of `cluster`.
- `sentry.utils.make_rb_cluster` in favor of retrieving clusters defined
  in configuration via the clsuter manager.

This also updates all tests to retrieve Redis clusters via the cluster
manager, instead of via cluster configuration or direct construction via
`rb.Cluster` or `StrictRedis`. This also consolidates all test cluster
setup and teardown to the test wrapper methods.

Resolves GH-2530 and references GH-2693.
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants