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

Adds RPM Build SPEC file, init script and example config file #41

Merged
merged 4 commits into from Jan 2, 2013

Conversation

tomponline
Copy link

This PR adds the following files

  • nutcracker.spec - an RPM spec file
  • nutcracker.init - an init script for starting nutcracker as a service in Redhat
  • nutcracker.yml - an example config file installed by the RPM, but marked as "confignoreplace" to allow modifications after installation

@manjuraj
Copy link
Collaborator

manjuraj commented Jan 2, 2013

Looks good overall :) Could you resubmit after incorporating the following comments:

  • move the changes from contrib/ folder to scripts/ folder as the contrib folder corresponds to 3rd party libraries
  • instead of including nutcracker.yml, could you just refer to nutcracker.yml in conf folder (Is that possible?)
  • use 4 spaces indentation

@tomponline
Copy link
Author

Hi, no problem I'll get those changes made.

Including the custom config from the source tarball will be fine, and I'll do the same for the init script too.

@tomponline
Copy link
Author

One thing I have noticed is that if the config file is wrong, or if the program cannot start for another reason, perhaps there is something already listening on the same port. The program does not seem to return a non-zero error code and so the init script says "Starting nutcracker OK", even though it has not started.

Is it possible for nutcracker to return non-zero?

@manjuraj
Copy link
Collaborator

manjuraj commented Jan 2, 2013

I fixed the non-zero exit status issue here: 67ad495

manjuraj pushed a commit that referenced this pull request Jan 2, 2013
add rpm build spec and init script
@manjuraj manjuraj merged commit 621a859 into twitter:master Jan 2, 2013
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.

None yet

2 participants