dune73 / test-4

2 stars 4 forks source link

Use inotifywait to watch for changes instead of reloading manually. #75

Open studersi opened 6 years ago

studersi commented 6 years ago

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

Requirements

Issues

dune73 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Yep, thought so.

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

studersi commented 6 years ago

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

studersi commented 6 years ago

[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 commented 6 years ago

This version of apachex should be fairly streamlined now.

The new features include:

This branch is now ready for merging.

studersi commented 6 years ago

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).