Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
It would be cool if the module allowed the specification of an alternative class for SearchApiSolrConnection. The apachesolr.module works this way, and it's been very helpful for us to make it work connecting to our Solr services on Pantheon.
I would love to offer search_api to Pantheon users. :)
Comments
Comment #1
tauno CreditAttribution: tauno commentedI was curious about how difficult this would be so I rolled an ugly 2-minute patch. It works fine, but probably isn't the cleanest approach. The patch uses the variable value for search_api_solr_connection_class as the class to use and defaults to the current class.
Comment #2
populist CreditAttribution: populist commentedHere is an updated patch based on the work tauno did that overrides the solr connection class. My patch uses a variable override (similar to the apache solr module) which I think is a little more generalized.
Comment #3
populist CreditAttribution: populist commentedAnd a quick reroll to do the operations in reverse.
Comment #4
tauno CreditAttribution: tauno commented#3 looks good and works in my basic test forcing a the solr server hostname.
@populist, thanks for jumping in. Now that I look at it, I'm pretty sure my patch was broken. At least it showed there weren't many changes necessary. Looking forward to search_api w/ solr hopefully being available on pantheon.
Comment #5
joshk CreditAttribution: joshk commentedI think we can actually do this even cleaner by overriding the Transport class. Took me a while to figure out as there's a fair amount of code in the class here that's been factored out (isn't used, but still in the file).
Comment #6
stopshinal CreditAttribution: stopshinal commentedHere is a rerolled patch to head, I cant confirm this works. Figure i'll toss this out here anyways.
Comment #7
stopshinal CreditAttribution: stopshinal commentedOk, well I had the chance to try this out, didn't work
Comment #8
stopshinal CreditAttribution: stopshinal commentedI'm getting this error when I try to apply: fatal: corrupt patch at line 62
Comment #9
populist CreditAttribution: populist commentedI was able to get the patch in #6 to apply cleanly to the latest dev release.
Comment #10
arrubiu CreditAttribution: arrubiu commentedI tried patch #6 with latest stable of the module but I can't connect to the solr server.
Also with -dev version
Comment #11
arrubiu CreditAttribution: arrubiu commentedNow it's working.
I've applied patch #3 with "patch filename.patch"
Comment #12
joshk CreditAttribution: joshk commentedLooks like there's a new RC2 out, which doens't include this patch and also doesn't work on Pantheon.
Any chance this flexibility can be more elegantly integrated?
Comment #13
drunken monkeySeems to work fine and I'm also fine with the approach. Can't really remember, but there seemed to be some reason against it, or I would've done it this way in the first place. Using a hardcoded class name seemed awefully bad practice.
Anyways, there are still a few issues that need to be addressed:
- The patch adds whitespace and formatting errors.
- The variable should be called
search_api_solr_connection_class
, it's not the service class we're replacing.- Unless I'm mistaken,
$class::method()
won't work in PHP 5.2 (but can't test and didn't find the docs right now). Please either test and confirm this works in PHP 5.2, or use a safer way to call static methods.- Note the variable's existence in README.txt (add an extra "Hidden configuration" header above "Developers").
- Delete the variable in
search_api_solr_uninstall()
.Comment #14
joshk CreditAttribution: joshk commentedI believe you're correct that the patch does in fact work against RC2. I'll take a fresh look this week. Thanks for the comments!
Comment #15
populist CreditAttribution: populist commentedHere is an updated patch that applies cleanly against RC2. It addresses the following considerations raised in #13:
-- changes name of variable to search_api_solr_connection_class
-- adds documentation and example code to README.txt
-- deletes the variable in the search_api_solr_uninstall()
It does not address the following issues:
-- refactoring the $class::method() to work cleanly with PHP 5.2.
Comment #16
joshk CreditAttribution: joshk commentedI can roll another version that uses
call_user_func()
to implement the class call. Done that workaround before. I'll post a new patch sometime later today or tomorrow.Comment #17
erics14 CreditAttribution: erics14 commentedJoshk, when do you expect the revised patch to be available?
Comment #18
dellad CreditAttribution: dellad commentedsubscribed
Comment #19
drewish CreditAttribution: drewish commenteddellad, you don't have to comment to subscribe any more. Just click the big Follow button at the top right of the page.
Comment #20
nafmarcus CreditAttribution: nafmarcus commentedCan I second #17?
I'm all ready to deploy Solr on my pantheon site using the search api module and would love to use a clean, Pantheon patch. Otherwise am I to assume I should use patch from #15?
Comment #21
nafmarcus CreditAttribution: nafmarcus commentedI installed the patch on my local machine, restarted my Solr server and if clear cache or to to /admin I am now receiving the following error:
Parse error: syntax error, unexpected T_PAAMAYIM_NEKUDOTAYIM in /Users/nafmarcus/GITB/itcs1/sites/all/modules/contrib/search_api_solr/service.inc on line 850
Do I need to configure some file by hand?
Comment #22
erics14 CreditAttribution: erics14 commentedRoll on the day when this is solved in RC3!
Comment #23
frobNot exactly sure why, but this patch doesn't apply at all.
Comment #24
joshk CreditAttribution: joshk commentedUpdated patch using call_user_func() to safely invoke a variable class name as per example #4 in the PHP docs. Also cleaned up some whitespace.
Comment #25
randallknutson CreditAttribution: randallknutson commentedI found a couple issues trying to use the patch in #24.
1. The latest version of pantheon_apachesolr (at least the latest that I'm aware of) actually sets the variable "search_api_solr_service_class" not "search_api_solr_connection_class" which took me a while to figure out why it wasn't working. (I'm sure Josh knows more about the variable name than I do but this is what is currently working on a fully patched pantheon site.)
2. There is a type around line 853 of service.inc which has the parenthesis in the wrong place
should be
I've attached a patch with these two changes and have tested it on pantheon and it works with the latest 7.14 from them with pantheon_apachesolr.
Comment #26
joshk CreditAttribution: joshk commentedThanks for that, Randall! Must have been an oversight on my end. :P
Comment #27
drunken monkeySee my comment in #13:
Also, the lines in README.txt shouldn't be longer than 80 characters, there were some whitespace issues and the text in the README should better explain what is replaced, instead of citing an example.
Please see if the attached patch is OK for you!
Comment #28
populist CreditAttribution: populist commentedI checked this out on Pantheon and this looks good to me!
It requires a change, of course, to our Pantheon Apache Solr connector module, but I created a branch for that with this commit (https://github.com/pantheon-systems/drops-7/commit/a936b4b7f86608f34d679...) to help folks on their way.
Comment #29
arrubiu CreditAttribution: arrubiu commentedSo to apply #27 I've to wait for the patch to pantheon apache solr?
Comment #30
joshk CreditAttribution: joshk commentedThe patch at #27 should work with Pantheon now.
Comment #31
arrubiu CreditAttribution: arrubiu commentedWith the last stable version of search_api_solr?
Comment #32
gmclelland CreditAttribution: gmclelland commented@sergio.durzu - The patch applies fine against 7.x-1.0-rc2
Comment #33
desmondmorris CreditAttribution: desmondmorris commentedIf you disable Pantheon Solr Search locally (to test with a local solr instance), you will get missing class errors for "PantheonSearchApiSolrService". I updated the patch from #27 and added a quick check to determine whether the class name stored in the search_api_solr_connection_class variable exists or not.
Comment #34
mkalbere CreditAttribution: mkalbere commentedWould this allow to use SolrPhpClient/apache/Solr/HttpTRansport/Curl.php , or it has nothing to do with this ?
By default search_api_solr use file_get_content to communicate with Solr. So it opens a LOTS of tcp connection. Using curl would allow to use "keep alive" connection. Is there a way to do that ?
Comment #35
pirog CreditAttribution: pirog commentedHere is a rerolled patch for the latest dev (RC2+13) with a couple of small tweaks because it looks like some of the code has changed significantly since #33.
Comment #36
pirog CreditAttribution: pirog commentedactually... im getting weird results with #35... so i think we need to do some more here
Comment #37
populist CreditAttribution: populist commentedHere is an updated version of the patch in #33 which addresses the latest release of RC3.
Comment #38
pirog CreditAttribution: pirog commentedtested this out on my local solr server, after updating the schema, and it seems to work fine
Comment #39
gmclelland CreditAttribution: gmclelland commentedJust tried the patch in #37 with 1.0-rc3.
With the Pantheon modules enabled, when clearing an index Locally on my computer I get this error on /admin/config/search/search_api/index/node_index/status:
Strict warning: Creating default object from empty value in PanteheonSearchApiSolrHttpTransport->performHttpRequest() (line 153 of /Users/glenn/websites/7c9d097f-0bc1-457d-8314-1d9e37f102bf/modules/pantheon/pantheon_apachesolr/Pantheon_Search_Api_Solr_Service.php).
I couldn't index my nodes until I disabled the Pantheon modules locally.
After the nodes were reindexed I ran a search with a search_api_solr view I got the following error that doesn't want to go away:
Warning: call_user_func() expects parameter 1 to be a valid callback, class 'PantheonSearchApiSolrService' not found in SearchApiSolrService->flattenKeys() (line 892 of /Users/glenn/websites/7c9d097f-0bc1-457d-8314-1d9e37f102bf/profiles/cmf/modules/contrib/search_api_solr/service.inc). (x2)
Let me know if you need any more info. I did clear the caches.
Note: the searches made with views and search_api_solr and search_api_autocomplete still function correctly.
Comment #40
joshk CreditAttribution: joshk commentedStill working on support on the platform for rc3. The transport method and schema both changed a lot. We hope to have something in the next couple days.
Comment #41
pirog CreditAttribution: pirog commentedyes. we were having the same problems with kalabox.
@joshk... would be interested in seeing what you guys end up doing so we can try to preserve kalabox/pantheon parity.
Comment #42
tim.plunkettNote, if you're trying to get this working on Pantheon right now, 7.x-1.0-rc3 does not work.
I'm not sure which commit broke it, but rolling all the way back to 7.x-1.0-rc2 works great with this patch (a reroll of #33).
Comment #43
ufku CreditAttribution: ufku commentedHere is the patch for RC3.
Additionally, PantheonSearchApiSolrService has some bug with POST method which is the default in RC3.
A workaround is to select "GET" as the "HTTP method" under solr server settings.
Comment #44
BarisW CreditAttribution: BarisW commentedufku; thanks for adding this patch. I will test it ASAP.
I'm wondering if you've tested autocompletion functionality, as the RC2 version doesn't seem to work.
Comment #45
gmclelland CreditAttribution: gmclelland commentedAnybody have success yet with RC3 on Pantheon and in a local install yet?
@joshk - any updates?
Comment #46
BarisW CreditAttribution: BarisW commentedThe patch doesn't seem to work. I cannot index anymore (localhost).
I've tried setting it to GET, no luck either.
Comment #47
BarisW CreditAttribution: BarisW commentedHmm, it seemed to be a variable still set to the old class.
Running
drush vset search_api_solr_connection_class SearchApiSolrConnection
fixed it.Local all seems to work fine. Will need to test it on Pantheon as well, so setting it back to Needs review.
Comment #48
BarisW CreditAttribution: BarisW commentedAlso tested on Pantheon, and works fine ;)
THe only missing on the patch is a hook_update_N() to set (or maybe delete?) the search_api_solr_connection_class variable.
However, on Pantheon, this variable should be set to PantheonSearchApiSolrService, so in this case Pantheon could add it to their settings.php?
Comment #49
BarisW CreditAttribution: BarisW commentedHmm, I think I was over-excited here. I thought all worked fine, but recently it stopped working (and I cannot point the cause of this). I've tried updating Search API to 1.4, but no luck. I believe this needs more and more proper testing.
Apologies for the miscommunication.
Comment #50
omega8cc CreditAttribution: omega8cc commentedFor reference, please check the other issue I just closed as a duplicate - #1844250: Broken Solr 1.4 compatibility results with error: The Solr server could not be reached. - More details:
Commerce Kickstart 2.5 *without* any patch - http://drupal.org/node/1844250#comment-7241138
Panopoly head with patch from #37 above - http://drupal.org/node/1844250#comment-7241192
Comment #51
drunken monkey#1846254: Remove the SolrPhpClient dependency has introduced some relevant changes, so this would need a re-roll. I think it'd be best to set the connection class to the variable at service class creation time (i.e., in
__construct()
), then it will automatically be used everywhere.We also introduced an interface for connection classes, which the README.txt regarding this feature should probably mention.
Comment #52
populist CreditAttribution: populist commentedAwesome to see the SolrPHPClient dependency removed. This simplified a *lot* of the work needed to automatically configure Solr on a platform.
The attached patch provides support for overriding the SearchApiSolrService class as part of the search_api_solr_search_api_service_info(). This is a hidden variable as documented by the README. The patch also provides hidden configuration for passing a custom stream wrapper context to drupal_http_request() which is entirely optional and is documented in the hidden configuration section fo the README.
Comment #53
drunken monkeyWouldn't this all also be possible with a simple
hook_search_api_service_info_alter()
implementation? I don't think the benefit warrants the additional complexity (even though its minimal).Comment #55
populist CreditAttribution: populist commentedThanks for mentioning this hook and that would indeed allow us to simplify this patch even further. In fact, I just turned this issue into a documentation task and have an attached patch to document the method suggested in #53 to change the connection class.
I also opened a related, but necessary issue, in #1990422: Provide Programatic Support for Stream Wrapper Contexts for Custom Connection Classes to provide programatic support for adding Stream Wrapper Contexts. This will allow folks to override the connection class using the documentation here and then pass along Stream Wrapper Contexts to makeHttpRequest in Search API Solr.
Comment #56
drunken monkeyNote that there's also a
setConnectionClass()
method on the service class, which allows you to do this without overriding the service class.Also, since overriding the service class is possible for all services with that hook, not just for Solr, I'm not sure we should document this separately here. Maybe just add a child page to the Search API documentation.
Comment #57
Tim Corkerton CreditAttribution: Tim Corkerton commentedi'm trying to use Search API with Pantheon and there's a note here http://helpdesk.getpantheon.com/customer/portal/articles/361249-apache-s... which basically says you need to use rc2 + a patch to use Search API on Pantheon. This was written before rc4 was released. Is is possible to get rc4 running with Pantheon? Do I need to apply the patch discussed above. Thanks
Comment #58
seanrI tried RC4 unsuccessfully. We really need to get a fix in for this - it's blocking a planned release (though we're still about a month away). Using an old and potentially buggy version with a patch is not an ideal solution.
Comment #59
Anonymous (not verified) CreditAttribution: Anonymous commented+1, same issue here. Still running patched RC2.
Comment #60
drunken monkeySince I haven't used or examined Pantheon, I don't really know what would have to happen for users to be able to use RC4 or later with Pantheon. It seems they use a variable to set the connection class, which we still don't allow by default – but perhaps, with this newer version, this wouldn't be needed anymore and they could do the same with an alter hook.
In any case, you'd have to contact the Pantheon support about that, I can't tell you for sure.
Comment #61
scor CreditAttribution: scor commentedIn case it helps, the connection class used by Pantheon is on githuh: https://github.com/pantheon-systems/drops-7/blob/master/modules/pantheon...
Comment #62
drunken monkeyUntil we hear from some Pantheon developer, I don't think we can do anything here. I agree with all the last posters, this should really be changed. RC 2 is now over a year old and there have been huge changes since then.
Comment #63
Anonymous (not verified) CreditAttribution: Anonymous commentedI am a Pantheon user. Pantheon developers released an updated Pantheon module 3 weeks ago that support the Search API Solr RC5, no patch required. I haven't done any extensive testing yet, but it seems to work properly.
Comment #64
drunken monkeyOh, excellent news, thanks for reporting this here!
Seems this issue is resolved, then. Or should we still change something?
Comment #65
colanIt would be great if a Pantheon user with a support plan could get in touch with them and tell them to update this page: http://helpdesk.getpantheon.com/customer/portal/articles/361249-enabling...
Until that happens, the page will keep referring folks over here, causing confusion.
Once that's updated, we can close this.