Due to the search base that is getting bigger everyday, we need a way to guide people to different load balancers.

This patch is doing exactly that

CommentFileSizeAuthor
#15 1716944-15.patch1.73 KBpwolanin
#14 1716944-14.patch1.73 KBpwolanin
#9 1716944-9.patch1.87 KBpwolanin
#8 1716944-8.patch1.53 KBpwolanin
#5 1716944-5.patch2.74 KBNick_vh
#4 1716944-3.patch2.77 KBNick_vh
#3 1716944-2.patch2.61 KBNick_vh
#1 1716944-1.patch1.89 KBNick_vh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Nick_vh’s picture

Nick_vh
he/him
Location Ghent
CreditAttribution: Nick_vh commented
FileSize
1716944-1.patch1.89 KB
pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Active » Needs work

maybe should have a different variable name:

+    'url' => $search_base_url . variable_get('acquia_search_url', '/solr/'. $identifier),

this is now the path not the URL?

Also - is this going to dynamically check that the server is right if the heartbeat data changes? It's not clear from the context in the patch.

Nick_vh’s picture

Nick_vh
he/him
Location Ghent
CreditAttribution: Nick_vh commented
FileSize
1716944-2.patch2.61 KB

Clarified the use of the variables and added an update function.

This does dynamically check and update the info on every heartbeat

/**
 * Implementation of hook_acquia_subscription_status().
 */
function acquia_search_acquia_subscription_status($active) {
  if ($active) {
    acquia_search_enable_acquia_solr_environment();
  }
  else {
    acquia_search_disable();
  }
}
Nick_vh’s picture

Nick_vh
he/him
Location Ghent
CreditAttribution: Nick_vh commented
Status: Needs work » Needs review
FileSize
1716944-3.patch2.77 KB

forgot to remove the correct variable.

Nick_vh’s picture

Nick_vh
he/him
Location Ghent
CreditAttribution: Nick_vh commented
FileSize
1716944-5.patch2.74 KB

Fixing the newline error

Nick_vh’s picture

Nick_vh
he/him
Location Ghent
CreditAttribution: Nick_vh commented
Status: Needs review » Fixed

Committed.

Backport issue here :
#1720712: Allow Acquia Search to insert alternative url

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Fixed » Active

I think the code here could be a little better - and also prepare us for better hosting integration.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Active » Needs review
FileSize
1716944-8.patch1.53 KB

This will let us also use the $conf['acquia_search_host'] currently populated in Acquia Cloud servers.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
FileSize
1716944-9.patch1.87 KB

ok, that didn't quite work as intended now, since the host in the RPC overwrites the 'internal.' host name.

Here's a patch that seems to be working for DevCloud.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs review » Fixed

committed - needs to be ported.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

committed some extra code comments too per Nick.

Status: Fixed » Closed (fixed)

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

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs work » Active

Given the certificate issues, we may want to use 'internal-' rather than 'internal.'

Note that with our current setup, we won't have peer certs for all possible host names - should we always fall back to search.acquia.com and assume the same CA will be used for all?

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Closed (fixed) » Needs work
FileSize
1716944-14.patch1.73 KB

leaving as "needs work" so we can consider the cert question

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Active » Needs work
FileSize
1716944-15.patch1.73 KB

Since this is checked infrequently, maybe we should match both . and - for the legacy check?

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs work » Needs review
cpliakas’s picture

cpliakas CreditAttribution: cpliakas commented

Yes, I think that is a sane approach. Would love to see some tests for this functionality as well. I am happy to volunteer writing them after the fact so that this code doesn't hold up a release.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs review » Fixed

committed. Follow-up for 6.x at: #1791684: Follow up to Allow Acquia Search to insert alternative url

Status: Fixed » Closed (fixed)

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