search

mattporritt / moodle-search_elastic

An Elasticsearch engine plugin for Moodle's Global Search
https://moodle.org/plugins/search_elastic
GNU General Public License v3.0
16 stars 13 forks source link

The api calls to elastic should honor the moodle proxy settings #9

Closed brendanheywood closed 7 years ago

brendanheywood commented 7 years ago

http://docs.guzzlephp.org/en/latest/request-options.html#proxy

ie it should honor the stuff found inside:

/admin/settings.php?section=http

mattporritt commented 7 years ago

Thanks Brendan, I see the need for this now. As you've suggested I'll get it to apply/respect the Moodle proxy settings. I think this is a better way than adding plugin specific proxy settings. DRY FTW

mattporritt commented 7 years ago

OK, I've created a branch for this called proxy. Which is very much a work in progress. The Guzzle docs are easy to follow, and it was pretty easy just to hack in a static proxy on the get request method to test. However, I'm having issues resolving the Guzzle implementation with the Moodle implementation. I don't have a setup available that actually uses Moodle's proxy settings. One of the things I'd like to know is in the moodle admin setings, what actually goes in the setting 'Proxy host' is it a domain like localhost or an actual URI like http://localhost. I'd also like to know how (if at all) Moodle deals with HTTPS proxies, the Moodle docs here aren't helpful and I haven't been able to figure it out from the code. I'll keep digging

ghost commented 7 years ago

10 Created pull request for basic HTTP proxy support. Confirmed functionality on our environment. This could be further extended to support username/password and perhaps proxy type.

AFAIK, only proxy machine hostname (or IP address) goes into $CFG->proxyhost field, without protocol definition.

mattporritt commented 7 years ago

This should be pretty much done now. I've updated the code to respect all of Moodle's HTTP proxy settings, including auth and bypass. There is unit test coverage of the functionality. I've also verified it's working using Squid as the proxy on my test rig. The only thing left to do is test it on a real setup, which I don't have at hand. @martindCatalyst are you able to pull the latest from the proxy branch and test it? If every thing checks out, I'll bump the version and merge it into master.

ghost commented 7 years ago

Hello @mattporritt On our environment with web servers not having access to elasticsearch instance, and using squid3 as HTTP proxy I can confirm this is working as expected.

All green from me.

Thank you for your work

mattporritt commented 7 years ago

Changes merged into master, new version released. Thanks all for your help.