- Notifications
You must be signed in to change notification settings - Fork 313
Added configuration options for custom redis servers as well as bin locations #35
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
Conversation
…ck is missing in config.yml
…ck is missing or empty in config.yml
| ++, although this patch doesn't cover the case where you want gitlab-shell to connect to redis via a unix socket (redis-cli -s /var/run/redis/redis.sock). Would it be possible to support that case as well? Thanks for the patch! |
| I'll include the socket parameter later tonight or tomorrow after I get my dev box backup. |
| I really appreciate this. Thanks! |
| It took a bit longer then I expected to get KVM running on my dev box. I am adding the patch right now. Hopefully you can test it since I don't have a local redis install on the same VM. Commit should be added within the next 20 or so minutes. |
| Whoops, I forgot to update the config.yml.example with the socket configuration option. Updated repository with the amended config.yml.example. |
lib/gitlab_update.rb Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a small bug in this version: "rpush '#{@redis['namespace']}:queue:post_receive'" should go into command instead of redis_command. Besides the code duplication, this also causes default configuration (redis.empty? == true) not to take in that command.
…nly called once in the update hook.
| Per your request, I've modified the update hook and fixed the typo. I've also refactored the initialize class to initialize only one copy of GitlabConfig instead of two. On a site note coding via nano is quite a pain. I should have caught that myself, but my dev vm with my full IDE is offline still. |
| Looks great now, I just applied it to my installation. Thanks :) @randx , could you please have a look? |
| +1 |
| Applied this to my gitlab 5.0 installation and it fixed the issue that commits was not posted to the dashboards. |
| Looks good for me. thank you |
Added configuration options for custom redis servers as well as bin locations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If @redis.empty? and we fallback to old connecting we got wrong behaviour here because @redis['namespace'] is nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'll submit a new pull request later this evening that will switch out the command used based on @redis existing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by c110d11





Gitlab-Shell currently has the path and server for Redis hard-coded within lib/gitlab_update.rb.
With this patch users will be able to modify the redis settings within config.yml to successfully display commit notices on their Dashboards since gitlab will pickup the commit notices from Sidekiq/Redis.
Offending line causing a headache as git commits were not being posted to the user and project dashboard:
command = "env -i redis-cli rpush 'resque:gitlab:queue:post_receive' '{\"class\":\"PostReceive\",\"args\":[\"#{@repo_path}\",\"#{@oldrev}\",\"#{@newrev}\",\"#{@refname}\",\"#{@key_id}\"]}' > /dev/null 2>&1"I've included the option to control the namespace, because it must be specified in gitlab's resque.yml + environments/.yml under cache as *:redis_store, "redis://redis.example.com:port/namespace**
Validated against ruby v1.9.3-p392 and rspec v2.12.0