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

add --pid-file-dir option for setting location of pid file. #146

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@suzuken
Contributor

suzuken commented Apr 10, 2014

Option for changing pid location. It's only enable when --daemon option is specified.

@sebdah

This comment has been minimized.

sebdah commented on dynamic_dynamodb/__init__.py in 357ccd9 Apr 10, 2014

This is not valid syntax. Should be something like:

pid_file = '{0}/dynamic-dynamodb.{1}.pid'.format(pid_file_dir, get_global_option('instance'))
@sebdah

This comment has been minimized.

sebdah commented on dynamic_dynamodb/__init__.py in 357ccd9 Apr 10, 2014

You are using an option called pid-file-dir, but that option is not available as a command line option. Maybe this commit is partial and misses some changes?

@sebdah

This comment has been minimized.

Owner

sebdah commented Apr 10, 2014

Thank you for the PR @suzuken. I have looked it through and it seems to be missing some parts to work. See my inline comments above.

I do think that this is a great idea and it would be great to get it implemented and merged.

@suzuken

This comment has been minimized.

Contributor

suzuken commented Apr 10, 2014

@sebdah Thank you for your quick reply and review. I'm not familiar with scripting in Python, so grad to see your commend.

Fixed some codes and add file for adding options. By the way, the docs is automatically generated by script and should I do this in my hand? If so, I'll add it on this PR.

@sebdah

This comment has been minimized.

Owner

sebdah commented Apr 10, 2014

Thanks! I will fix merging with the latest code base and also write the docs. I'll also make a few changes to make the command line this work (there's a global option that must be set and referenced properly).

Will fix that and update here soon.

@sebdah sebdah added this to the 1.11.x milestone Apr 10, 2014

@sebdah sebdah self-assigned this Apr 10, 2014

sebdah added a commit that referenced this pull request Apr 10, 2014

sebdah added a commit that referenced this pull request Apr 10, 2014

@sebdah

This comment has been minimized.

Owner

sebdah commented Apr 10, 2014

Thanks again @suzuken! I have now fixed the mentioned things and the code has been merged to the develop branch, meaning that it will be included in the next release of Dynamic DynamoDB. I intend to release the next version sometime within this week.

@sebdah sebdah closed this Apr 10, 2014

@suzuken

This comment has been minimized.

Contributor

suzuken commented Apr 10, 2014

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment