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 /tmp for default pid dir #22

Merged
merged 3 commits into from
Feb 23, 2016

Conversation

muratayusuke
Copy link
Contributor

I faced same issue as this one.

It's because old pid file is in old release dir, but restart command searches new release dir.

I fixed this issue by passing /tmp dir for --pid-dir option. It's working fine for me.

@blackjid
Copy link
Member

That would be one way to solve it...
but I think is the same issue described here https://github.com/platanus/capistrano3-delayed-job/issues/16#issuecomment-159411885

or I'm missing something?

@muratayusuke
Copy link
Contributor Author

Yes, having set :linked_dirs, %w{tmp/pids} also solve it.

I think adding --pid-dir have no problem, but changing default value is a little aggressive.
I prefer using /tmp dir because no additional setting is required.
How do you think about it?

Anyway, I reverted renaming :delayed_log_dir. It breaks compatibility, so I think it's not good to mix here.

@blackjid
Copy link
Member

Ok, it make sense to have this option if delayed jobs provide it for us...

I would prefer if the default value is nil so the behaviour is not changed.
Also if you can add this new option to the readme it would be awesome. It could be in the main options section but it may also deserves some extra explanation where we show the need to set: linked_dirs, %w(tmp/pids) or to use something like set :delayed_pid_dir, '/tmp'

With that I'd be happy to merge this PR

@muratayusuke
Copy link
Contributor Author

Thanks, I pushed that changes. It would be nice if everything works fine without any extra configuration from next major version :)


set :delayed_job_pid_dir, '/tmp'
```

## Contributing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect... thanks!

blackjid added a commit that referenced this pull request Feb 23, 2016
@blackjid blackjid merged commit 9728468 into platanus:master Feb 23, 2016
@muratayusuke muratayusuke deleted the feature/pid_dir_option branch February 23, 2016 02:12
@blackjid
Copy link
Member

thanks @muratayusuke I relaeased in 1.6.0

@muratayusuke
Copy link
Contributor Author

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants