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

Fix SEARCH_X_ACCEL_REDIRECT to work with solr #600

Merged
merged 1 commit into from Jan 16, 2018

Conversation

samj1912
Copy link
Collaborator

@samj1912 samj1912 requested review from zas and mwiencek January 15, 2018 18:12
@mwiencek
Copy link
Member

Can you explain? There's no /internal/solr/ rule in the linked nginx config (and I don't understand why you would need a separate one in any case).

$url_ext = "/ws/2/$resource/?" .
"max=$limit&type=$resource&fmt=$format&offset=$offset" .
"&query=" . uri_escape_utf8($query) . "&dismax=$dismax";
} else {
my $format = ($args->{fmt} // "") eq "json" ? "mbjson" : "mbxml";
my $def_type = $dismax eq 'true' ? 'dismax' : 'lucene';
$redirect_url = '/internal/solr/'
$url_ext = "/$resource/select?" .
"rows=$limit&wt=$format&start=$offset" .
"&q=" . uri_escape_utf8($query) . "&defType=$def_type";
}

if (DBDefs->SEARCH_X_ACCEL_REDIRECT) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the trick is only related to X_ACCEL thing, i would keep the code together:

my $redirect_url = (DBDefs->SEARCH_ENGINE eq 'LUCENE') ? '/internal/search/' : '/internal/solr/';
return { redirect_url => $redirect_url . DBDefs->SEARCH_SERVER . $url_ext }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also unbreaks the code (missing semicolons :)).

@zas
Copy link
Contributor

zas commented Jan 15, 2018

Can you explain? There's no /internal/solr/ rule in the linked nginx config (and I don't understand why you would need a separate one in any case).

I added one, not yet committed, as we need to ensure it works first

location ~* ^/internal/solr/(.*?)/solr/(.*) {
  internal;

  # Extract url from the request
  set $search_uri $2;
  set $search_host $1;

  # Compose url
  set $search_url http://$search_host/solr/$search_uri?$args;

  proxy_pass $search_url;

  add_header Access-Control-Allow-Origin *;
}

I'm not happy with this solution, but we just need it to work at this point.

@mwiencek mwiencek merged commit 2638cef into metabrainz:master Jan 16, 2018
@samj1912 samj1912 deleted the solr branch February 9, 2018 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants