We have a number of indexes hosted at websolr.com, for various reasons they do not allow access to the Solr administration interface.

Previously this was not a problem, but with the latest version of seach_api_solr schema-version and solr-spec-version is automatically determined by fetching some metadata from the admin interfaces. As Websolr respons with a 401 the following check in solr_connection.inc blows up

protected function checkResponse($response) {
    $code = (int) $response->code;

    if ($code != 200) {
      if ($code >= 400 && $code != 403 && $code != 404) {
        // Add details, like Solr's exception message.
        $response->status_message .= $response->data;
      }
      throw new SearchApiException('"' . $code . '" Status: ' . $response->status_message);
    }

    return $response;
  }

The same issue has been filed over at apachesolr: https://drupal.org/node/1956636

One "fix" would be to make it possible to disable the automatic checkking, but as I quite like that the versions are exposed in the backend, I've instead added two configuration options under the advanced server configuration to make it possible to override the schema-version and solr-spec-versions.

It might be that another approach makes more sense, but this at least fixes the issue provided that you are OK with specifying the two versions.

The patch is done against 7.x-1.x, but it applies cleanly to 1.4 as well.

CommentFileSizeAuthor
#6 override-schema-and-solr-spec-2175829-6.patch4.63 KBdanquah
#4 override-schema-and-solr-spec-2175829-4.patch4.65 KBdanquah
#4 searchapi-solr-exception.png264.38 KBdanquah
#4 missing-schema.png204.05 KBdanquah
#1 override-schema-and-solr-spec-2175829.patch2.96 KBdanquah

Comments

danquah’s picture

danquah commented
StatusFileSize
new override-schema-and-solr-spec-2175829.patch2.96 KB
danquah’s picture

danquah commented
Status: Active » Needs review
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Needs work

Thanks for posting this issue, and already providing a patch!

Previously this was not a problem, but with the latest version of seach_api_solr schema-version and solr-spec-version is automatically determined by fetching some metadata from the admin interfaces. As Websolr respons with a 401 the following check in solr_connection.inc blows up

What do you mean, "blows up"? For me this just prints a clean error message along with the rest of the information. Really nothing dramatic, and not a bad solution I'd say.
However, not knowing the Solr version in this case is of course unfortunate, as we do use that internally to determine some settings. So having the option to override the automatic retrieval here makes of course sense and is a nice solution to the problem. I would just make it a select field, though, of 1, 3 or 4. (Maybe with "1.4" as the label for "1", and "3.x"/"4.x" for the others). We don't need the version in more detail, and a select field is much more comfortable to use.

Also, I don't see why it would make sense to also allow users to define the schema version. We don't use it internally in any way, so the only use this would have is to display the schema version they have entered right back to them. If they really want to, they can just use the server description for that.
Or am I missing something here?

In any case, giving the user the option to specify the version of Solr they are running is really a great idea, thanks again! Could you maybe re-roll the patch with my suggestions?

danquah’s picture

danquah commented
Status: Needs work » Needs review
StatusFileSize
new missing-schema.png204.05 KB
new searchapi-solr-exception.png264.38 KB
new override-schema-and-solr-spec-2175829-4.patch4.65 KB

Thank you for the input.

OK, "blows up" was a bit colorful :) I have attached an image of the configuration-page as it looked with an unpatched search_api_solr, it could be a bit prettier, on the other hand its quite clear that some error occurred.

The reason I adde the schema-name override is that the schema-version is checked for incompatibility in the following block from around line 425 in service.inc

            if (substr($stats_summary['@schema_version'], 0, 10) == 'search-api') {
              drupal_set_message(t('Your schema.xml version is too old. Please replace all configuration files with the ones packaged with this module and re-index you data.'), 'error');
              $status = 'error';
            }
            elseif (substr($stats_summary['@schema_version'], 0, 9) != 'drupal-4.') {
              $variables['@url'] = url(drupal_get_path('module', 'search_api_solr') . '/INSTALL.txt');
              $message = t('You are using an incompatible schema.xml configuration file. Please follow the instructions in the <a href="@url">INSTALL.txt</a> file for setting up Solr.', $variables);
              drupal_set_message($message, 'error');
              $status = 'error';
            }

As far as I can tell it checks to see if solr is configured with the schema.xml bundled with search_api_solr.

I just realized that what you really want is a possibility to disable the check in the situation where we can either not retrieve the schema-name (as in this case) - or the name for some reason has to have an "incompatible" value.

I have rerolled the patch with your suggestion for a drop select-field, and changed the schema-name override to be a boolean "schema-check" override instead.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Needs work

Thanks for reworking your patch, looks quite good already. There are just a few minor issues left:

  1. +++ b/includes/service.inc
@@ -79,6 +79,8 @@ class SearchApiSolrService extends SearchApiAbstractService {
+      'schema_check_override' => '',

    The default here should be FALSE, since schema_check_override is a boolean and not a string value. Also, check_schema might be a better name. 

  2. +++ b/includes/service.inc
@@ -208,6 +210,24 @@ class SearchApiSolrService extends SearchApiAbstractService {
+        '' => '- Choose -',

    That should definitely be wrapped in a t() call. Also, "Determine automatically" or the like would be a better label, given the functionality. 

  3. +++ b/includes/service.inc
@@ -208,6 +210,24 @@ class SearchApiSolrService extends SearchApiAbstractService {
+      '#description' => t("Specify the SOLR version manually in case it cannot be retrived automatically. The version can be found in the SOLR admin interface under \"Solr Specification Version\" or \"solr-spec\""),

@@ -402,16 +422,19 @@ class SearchApiSolrService extends SearchApiAbstractService {
+                $status = 'error';

    You should use single quotes where possible, as per the coding standards. In this case, they would also eliminate the need to escape the included double quotes, which is even more important.

Other than that, I'd be happy to commit this. (Even though we should probably think at some point about how to better structure this settings page – it's getting a bit messy there.)

danquah’s picture

danquah commented
Status: Needs work » Needs review
StatusFileSize
new override-schema-and-solr-spec-2175829-6.patch4.63 KB

Good points! - I have updated the patch.

And yes, the settings page could probably use some love :)

  • Commit 59df963 on 7.x-1.x authored by danquah, committed by drunken monkey: 
    Issue #2175829 by danquah, drunken monkey: Fixed error when admin...
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

Excellent, thanks a lot!
Made another few corrections and then committed.
Thanks again for your work!

Status: Fixed » Closed (fixed)

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