Just a simple proposal: Add a apachesolr_server_status() function that encapsulates the ping functionality used for apachesolr_requirements().
It would a) clean up the code a bit and b) allow developers of related modules (like me) to simply use that function, instead of manually copying the code. Nothing fancy or vital, but maybe helpful, at least to me.

CommentFileSizeAuthor
#2 server_status-function.patch2.39 KBdrunken monkey
#2 server_status-function--no-iff.patch2.41 KBdrunken monkey
server_status-function.patch2.4 KBdrunken monkey

Comments

robertdouglass’s picture

robertdouglass commented
Status: Needs review » Needs work

+ watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);

you only want to use nl2br when doing drupal_set_message, not for watchdog.

There's a spelling error (iff -> if)

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review
StatusFileSize
new server_status-function--no-iff.patch2.41 KB
new server_status-function.patch2.39 KB

you only want to use nl2br when doing drupal_set_message, not for watchdog.

Thanks for the tip, corrected that.
Althoug it is worth mentioning that I only copied that from the apachesolr_requirements() function. The same code is also used at many other places in the module:

$ grep -rn nl2br . | grep watchdog
./apachesolr.admin.inc:119:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.admin.inc:133:      watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.admin.inc:157:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.admin.inc:178:      watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.admin.inc:407:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr_search.admin.inc:20:    watchdog('apachesolr', nl2br(check_plain($e->getMessage())));
./apachesolr_search.module:133:        watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr_search.module:918:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.module:332:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.module:397:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.module:461:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())) . ' in apachesolr_cron', NULL, WATCHDOG_ERROR);
./apachesolr.module:528:    watchdog('apachesolr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.module:1062:      watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.module:1133:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR);
./apachesolr.module:1338:    watchdog('Apache Solr', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_ERROR );
There's a spelling error (iff -> if)

It's not an error, "iff" is a (as far as I know rather common) short form of "if, and only if". But if it isn't as well-known as I thought, we can of course leave it out. In that case, use the second patch.

robertdouglass’s picture

robertdouglass commented

Oh, I didn't know that! iff

robertdouglass’s picture

robertdouglass commented
Status: Needs review » Fixed

Committed, thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.