Skip to content

Conversation

@7heo
Copy link
Contributor

@7heo 7heo commented Mar 24, 2017

Minor cosmetic change, for documentation by code mostly. Also, the README.md is corrected about the nohup.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 31.465% when pulling a89b548 on 7heo:master into 3988701 on ccnmtl:master.

@thraxil
Copy link
Contributor

thraxil commented Mar 24, 2017

FWIW, the run.sh script is just what I use when I'm testing locally and want to check something out. I wouldn't recommend using for any sort of production purposes (we deploy hound in a docker container). I'd actually prefer to not have a nohup in there, as I'd rather be able to just C-c it.

@7heo
Copy link
Contributor Author

7heo commented Mar 24, 2017

Here is about docker in production: https://thehftguy.com/2016/11/01/docker-in-production-an-history-of-failure/ and https://thehftguy.com/2017/02/23/docker-in-production-an-update/. Some people will still use docker in production, and some people will also never write tests. To each their own. Long story short, run.sh shouldn't be used to run locally, since the program is configured via environment variables, it should be the default way to run your software.

@thraxil
Copy link
Contributor

thraxil commented Mar 24, 2017

I'm just saying, the intention of the run.sh that's in the repo was not to start up a production instance, it was so I could test things locally. That's why it's pointing at a test config and a test graphite instance. It would be better to have it named dev.sh or something perhaps.

Hound itself is just a single binary file, that takes config from environment variables, so it's easy to run in a container, on Heroku, or started directly from systemd or supervisord or from a shell script or whatever fits one's own environment.

@7heo
Copy link
Contributor Author

7heo commented Mar 24, 2017

thraxil commented:

Hound itself is just a single binary file, that takes config from environment variables, so it's easy to run [...] from a shell script or whatever fits one's own environment.

I agree. I don't see why it's such a big deal to provide a working shell script, though (and do not forget that simple code like run.sh is a perfectly valid example, or maybe even documentation).

@7heo
Copy link
Contributor Author

7heo commented Mar 24, 2017

I have added some documentation for running it with docker, first. I do not pretend for it to be perfect or complete, but since I am not an avid docker user myself, it would be hard for me to do much better.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 31.465% when pulling cd4ff9f on 7heo:master into 3988701 on ccnmtl:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 31.465% when pulling 35c3481 on 7heo:master into 3988701 on ccnmtl:master.

@thraxil
Copy link
Contributor

thraxil commented Mar 29, 2017

With a nohup in the script, I can no longer just do run.sh in the terminal while I'm developing, look at what's printing out and C-c when I'm done. With nohup in there, I have to start it, then tail a log file, then C-c that, then track down the pid of the process and kill it. Since the whole point of run.sh was for developing locally, it makes it less useful for that.

@7heo
Copy link
Contributor Author

7heo commented Mar 29, 2017

I see. That is a valid point. Would you mind using an extra argument, or a different file for the development purpose? Such as ./run.sh dev or ./dev-run?

@thraxil
Copy link
Contributor

thraxil commented Mar 29, 2017

I think what I'm going to do is rename it to dev.sh so it's less confusing to new folks, then create an examples directory where example scripts and configs can live.

@thraxil thraxil merged commit 5040b6f into ccnmtl:master Mar 29, 2017
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.

3 participants