Description
Tomás Cohen Arazi (tcohen)
2016-10-19 12:44:28 UTC
Created attachment 56662 [details] [review] Bug 17467: Add koha-zebra script to handle Zebra daemons This script is intended to replace the following currently used scripts on packages setups: - koha-start-zebra - koha-stop-zebra - koha-restart-zebra It also introduces a --status option switch, for asking for daemon statuses To test - Apply the patch - Run: $ sudo debian/scripts/koha-zebra --start kohadev => SUCCESS: same behaviour than koha-start-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --stop kohadev => SUCCESS: same behaviour as koha-stop-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --restart kohadev => SUCCESS: same behaviour than koha-restart-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --status kohadev => SUCCESS: It correctly shows the status for the running (or not) process Play with different combinations of this commands Created attachment 56663 [details] [review] Bug 17467: (followup) Make koha-zebra handle legacy script names This patch adds the new koha-zebra script the ability to handle being called by other script names, for backwards compatibility with the legacy koha-*-zebra scripts. It is intended to be used in conjunction with a patch that replaces the old koha-*-zebra scripts for symbolic links, pointing to the new koha-zebra script. To test: - Apply this patch - Create symbolic links to the new script: vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-start-zebra vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-stop-zebra vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-restart-zebra - Run: $ sudo ./koha-start-zebra kohadev => SUCCESS: Same behaviour than the koha-start-zebra script - Run: sudo ./koha-stop-zebra kohadev => SUCCESS: Same behaviour than the koha-stop-zebra script - Run: sudo ./koha-restart-zebra kohadev => SUCCESS: Same behaviour than the koha-restart-zebra script Created attachment 56664 [details] [review] Bug 17467: Add koha-zebra documentation Created attachment 56665 [details] [review] Bug 17467: (followup) Make the install script install koha-zebra Created attachment 56669 [details] [review] Bug 17467: Add tab-completion for koha-zebra This patch adds tab-completion for bash, for the koha-zebra script. To test: - Run (on the vagrant user): $ source kohaclone/debian/koha-common.bash-completion - Run: $ sudo koha-zebra -<TAB> (<TAB> means press the tab key) => SUCCESS: Tab completion works as expected for the koha-zebra command. - Sign off :-D I started to test these patches: after run sudo debian/scripts/koha-zebra --status kohadev i get: Error: invalid option switch (--status) everything other works as expected (In reply to Josef Moravec from comment #6) > I started to test these patches: > > after run > sudo debian/scripts/koha-zebra --status kohadev > i get: > Error: invalid option switch (--status) > > everything other works as expected Thanks, Josef. It seems that I forgot to add it Created attachment 56693 [details] [review] Bug 17467: Add missing --status switch handling Comment on attachment 56662 [details] [review] Bug 17467: Add koha-zebra script to handle Zebra daemons Review of attachment 56662 [details] [review]: ----------------------------------------------------------------- ::: debian/scripts/koha-zebra @@ +135,5 @@ > + > + if is_zebra_running ${name}; then > + > + stop_zebra ${name} && \ > + start_zebra ${name} This logic is failing. The stop always runs, but the start never does. Remember 0 success, but 0 && foo is short-circuited logic. Also, missing verbose check to print a nice informational message about Restarting Zebra. @@ +148,5 @@ > +zebra_status() > +{ > + local name=$1 > + > + if is_zebra_running; then missing $name. Also, while I understand the change of logic makes sense in terms of the verbose switch, the script before defaulted to verbose, so perhaps the switch should be silent? That's not a Failed QA issue for me, but the ones in comment #9 are. Created attachment 58550 [details] [review] Bug 17467: Restart failing, missing instance parameter This follow up should make it work. Perhaps there is a more elegant way, but wanted to generate something that works. Before this patch, the --restart stops Zebra, but does not restart it. After, it does restart. Before this patch, the --status gives nasty long help. After this patch, reasonable output. What about adding dev_install here now, Tomas? Created attachment 66420 [details] [review] [SIGNED-OFF] Bug 17467: Add koha-zebra script to handle Zebra daemons This script is intended to replace the following currently used scripts on packages setups: - koha-start-zebra - koha-stop-zebra - koha-restart-zebra It also introduces a --status option switch, for asking for daemon statuses To test - Apply the patch - Run: $ sudo debian/scripts/koha-zebra --start kohadev => SUCCESS: same behaviour than koha-start-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --stop kohadev => SUCCESS: same behaviour as koha-stop-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --restart kohadev => SUCCESS: same behaviour than koha-restart-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --status kohadev => SUCCESS: It correctly shows the status for the running (or not) process Play with different combinations of this commands Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 66421 [details] [review] [SIGNED-OFF] Bug 17467: (followup) Make koha-zebra handle legacy script names This patch adds the new koha-zebra script the ability to handle being called by other script names, for backwards compatibility with the legacy koha-*-zebra scripts. It is intended to be used in conjunction with a patch that replaces the old koha-*-zebra scripts for symbolic links, pointing to the new koha-zebra script. To test: - Apply this patch - Create symbolic links to the new script: vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-start-zebra vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-stop-zebra vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-restart-zebra - Run: $ sudo ./koha-start-zebra kohadev => SUCCESS: Same behaviour than the koha-start-zebra script - Run: sudo ./koha-stop-zebra kohadev => SUCCESS: Same behaviour than the koha-stop-zebra script - Run: sudo ./koha-restart-zebra kohadev => SUCCESS: Same behaviour than the koha-restart-zebra script Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 66422 [details] [review] [SIGNED-OFF] Bug 17467: Add koha-zebra documentation Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 66423 [details] [review] [SIGNED-OFF] Bug 17467: (followup) Make the install script install koha-zebra Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 66424 [details] [review] [SIGNED-OFF] Bug 17467: Add tab-completion for koha-zebra This patch adds tab-completion for bash, for the koha-zebra script. To test: - Run (on the vagrant user): $ source kohaclone/debian/koha-common.bash-completion - Run: $ sudo koha-zebra -<TAB> (<TAB> means press the tab key) => SUCCESS: Tab completion works as expected for the koha-zebra command. - Sign off :-D Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 66425 [details] [review] [SIGNED-OFF] Bug 17467: Add missing --status switch handling Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 66426 [details] [review] [SIGNED-OFF] Bug 17467: Restart failing, missing instance parameter This follow up should make it work. Perhaps there is a more elegant way, but wanted to generate something that works. Before this patch, the --restart stops Zebra, but does not restart it. After, it does restart. Before this patch, the --status gives nasty long help. After this patch, reasonable output. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Tomas, this patch does (indeed) not need dev_install/adjust_paths_dev_install since it does not refer to KOHA_HOME or PERL5LIB. Created attachment 68623 [details] [review] Bug 17467: Add koha-zebra script to handle Zebra daemons This script is intended to replace the following currently used scripts on packages setups: - koha-start-zebra - koha-stop-zebra - koha-restart-zebra It also introduces a --status option switch, for asking for daemon statuses To test - Apply the patch - Run: $ sudo debian/scripts/koha-zebra --start kohadev => SUCCESS: same behaviour than koha-start-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --stop kohadev => SUCCESS: same behaviour as koha-stop-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --restart kohadev => SUCCESS: same behaviour than koha-restart-zebra kohadev - Run: $ sudo debian/scripts/koha-zebra --status kohadev => SUCCESS: It correctly shows the status for the running (or not) process Play with different combinations of this commands Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 68624 [details] [review] Bug 17467: (followup) Make koha-zebra handle legacy script names This patch adds the new koha-zebra script the ability to handle being called by other script names, for backwards compatibility with the legacy koha-*-zebra scripts. It is intended to be used in conjunction with a patch that replaces the old koha-*-zebra scripts for symbolic links, pointing to the new koha-zebra script. To test: - Apply this patch - Create symbolic links to the new script: vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-start-zebra vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-stop-zebra vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-restart-zebra - Run: $ sudo ./koha-start-zebra kohadev => SUCCESS: Same behaviour than the koha-start-zebra script - Run: sudo ./koha-stop-zebra kohadev => SUCCESS: Same behaviour than the koha-stop-zebra script - Run: sudo ./koha-restart-zebra kohadev => SUCCESS: Same behaviour than the koha-restart-zebra script Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 68625 [details] [review] Bug 17467: Add koha-zebra documentation Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 68626 [details] [review] Bug 17467: (followup) Make the install script install koha-zebra Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 68627 [details] [review] Bug 17467: Add tab-completion for koha-zebra This patch adds tab-completion for bash, for the koha-zebra script. To test: - Run (on the vagrant user): $ source kohaclone/debian/koha-common.bash-completion - Run: $ sudo koha-zebra -<TAB> (<TAB> means press the tab key) => SUCCESS: Tab completion works as expected for the koha-zebra command. - Sign off :-D Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 68628 [details] [review] Bug 17467: Add missing --status switch handling Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 68629 [details] [review] Bug 17467: Restart failing, missing instance parameter This follow up should make it work. Perhaps there is a more elegant way, but wanted to generate something that works. Before this patch, the --restart stops Zebra, but does not restart it. After, it does restart. Before this patch, the --status gives nasty long help. After this patch, reasonable output. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 17.11, thanks to everybody involved! Tomas, please add a text to go to the release notes. (In reply to Nick Clemens from comment #26) > Created attachment 68628 [details] [review] [review] > Bug 17467: Add missing --status switch handling > > Signed-off-by: Josef Moravec <josef.moravec@gmail.com> > > Signed-off-by: Nick Clemens <nick@bywatersolutions.com> I missed that one! Pushed to master for 18.05 Awesome work all! Pushed missed patch to Stable for 17.11.02 Enhancement not pushed to 17.05.x |