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 inotifywait to watch for changes instead of reloading manually. #75

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

studersi
Copy link
Contributor

Can you try out this version of apachex to see if it works for you?

Requirements

  • You will need to install the inotify-tools package for it to work

Issues

  • It is not possible to manually restart the server by pressing Enter anymore
  • Exiting with Ctrl+C does not make a clean shutdown like the entering q did before

@dune73
Copy link
Owner

dune73 commented Apr 23, 2018

Thanks. Looks like fun.

I have a few issues with this PR, though. Please fix these.

Dependency check

The error message for missing inotify package is rather ugly. There should be a dependency check with a nice error message and the help as well as the inline documentation should explain the dependency.

Ctrl-C combination

The Ctrl-C combination is only explained from the 2nd call

Missing config check

If you make an error in the config and save it, the server is stopped and can no longer be started.
use the binary's config test to check prior to stopping the running server

Verbosity and transparence

The script ought to be more transparent and verbose with what it does. I have something in mind like the following.

...
14:23:22 Setting up inotify watches
14:23:22 Entering watch mode
14:24:15 File has been changed. Checking config ... ok. Restarting ...
14:25:15 Stopping active apache process ... ok
14:25:16 Launching apache config file '/apache/conf/httpd.conf_pod_2018-04-23_14:38' ... ok
14:25:16 Edit and save file to restart apache, press [Ctrl+C] to exit.
14:25:22 File has been changed. Checking config ... FAIL.
14:25:22 Error message: Invalid command 'SrcRule', perhaps misspelled or defined by a module not included in the server configuration
...

You can also make this optional via a -v switch.

@studersi
Copy link
Contributor Author

Good suggestions.

The current version is more of a PoC than anything else but now that it appears to work, I'll look into streamlining it as you suggested.

I was even thinking of adding a manual mode (-m switch) in case it's run on a system where the user doesn't have permissions to install the inotify package.

@dune73
Copy link
Owner

dune73 commented Apr 23, 2018

Yep, thought so.

I like the -m switch idea to revert back to the former behaviour.

@studersi
Copy link
Contributor Author

Great, I'll start working on it some time this week.

@studersi
Copy link
Contributor Author

studersi commented May 8, 2018

[x] It is not possible to manually restart the server by pressing Enter anymore. (See ca1cf17)
[x] Exiting with Ctrl+c does not make a clean shutdown like the entering q did before. (See 0b184b9)
[x] The error message for missing inotify package is rather ugly. There should be a dependency check with a nice error message and the help as well as the inline documentation should explain the dependency. (See 0c33564)
[x] The Ctrl-C combination is only explained from the 2nd call. (See 9d09507)
[x] If you make an error in the config and save it, the server is stopped and can no longer be started.
use the binary's config test to check prior to stopping the running server. (See b0bf81f)
[x] The script ought to be more transparent and verbose with what it does. I have something in mind like the following. (See ad6bb86)

(This checklist will be updated according to progress)

@studersi
Copy link
Contributor Author

studersi commented May 8, 2018

This version of apachex should be fairly streamlined now.

The new features include:

  • A manual mode that can be reached with the -m flag (apachex -m). In manual mode, no inotify tools are required and apache is restarted by pressing enter.
  • In both modes, the program can be exited cleanly by pressing Ctrl+c. The program traps the signal and executes the shutdown code that was previously used.
  • In case the inotify tools are not installed and the program is run in normal mode, a message is printed and the program exits.
  • If there is an error in the config file, apache is not restarted and the error is printed (output of httpd -t).
  • There are now three verbosity levels that can be reached with 0 to 2 -v flags.
    • apachex: Minimal output necessary to understand what is happening. It is very similar as before.
    • apachex -v: Additional output that makes it easier to debug what the program is doing.
    • apachex -v -v: In addition to the previous verbosity level, timestamps are shown for each output. Note that the way command line arguments are parsed apachex -vv does not work.

This branch is now ready for merging.

@studersi
Copy link
Contributor Author

studersi commented May 8, 2018

I refactored the argument parsing a bit. It is now possible to use -vv instead of -v -v but it is still not possible to do -vm, they have to be passed as -v -m.

Also, I added the flag -c to specify an apache config file to be used (i.e. apachex -c conf/httpd.conf).

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.

2 participants