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

search pid files in correct directory #12

Merged
merged 2 commits into from
Apr 3, 2014
Merged

search pid files in correct directory #12

merged 2 commits into from
Apr 3, 2014

Conversation

levinalex
Copy link
Contributor

this should fix #9, (sidekiq:stop and sidekiq:cleanup)

fixes sidekiq:stop and sidekiq:cleanup
@seuros
Copy link
Owner

seuros commented Apr 3, 2014

pid_file is the absolute path to the pidfile.

@levinalex
Copy link
Contributor Author

strange. It isn't on my machine:

bundle exec cap production sidekiq:stop
DEBUG [61ad35f0] Running /usr/bin/env [ -f tmp/pids/sidekiq-3.pid ] on host.example.com
DEBUG [61ad35f0] Command: [ -f tmp/pids/sidekiq-3.pid ]
...
$ cat Gemfile.lock | grep capistrano
  remote: git://github.com/seuros/capistrano-sidekiq.git
    capistrano-sidekiq (0.2.0)
      capistrano
    capistrano (3.1.0)
    capistrano-bundler (1.1.2)
      capistrano (~> 3.0)
    capistrano-rails (1.1.1)
      capistrano (~> 3.1)
      capistrano-bundler (~> 1.1)
  capistrano (>= 3.0.0)
  capistrano-bundler
  capistrano-rails
  capistrano-sidekiq!
$ cat config/deploy.rb | grep sidekiq_pid
set :sidekiq_pid, "tmp/pids/sidekiq.pid"

@levinalex
Copy link
Contributor Author

... and the last one answers the question why it isn't

@seuros
Copy link
Owner

seuros commented Apr 3, 2014

why do you set it ?

remove that line : set :sidekiq_pid, "tmp/pids/sidekiq.pid" and try again.

@levinalex
Copy link
Contributor Author

yup. doing that. that helps. sorry to have wasted your time.

does this mean these changes here are a bad idea though? the way within current_path was sprinkled throughout the file seems a bit random.

@seuros
Copy link
Owner

seuros commented Apr 3, 2014

The changes are very good. Could you rebase and stash your commits ?

@levinalex
Copy link
Contributor Author

will do.

for_each_process changes into current_dir before yielding pid files
this fixes sidekiq:stop when sidekiq_pid is set to a relative path
seuros added a commit that referenced this pull request Apr 3, 2014
search pid files in correct directory
@seuros seuros merged commit 6b60c06 into seuros:master Apr 3, 2014
@seuros
Copy link
Owner

seuros commented Apr 3, 2014

Thank you

@levinalex
Copy link
Contributor Author

thanks 👍

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.

Keeps failing to stop sidekiq
2 participants