On Koha::SearchEngine::Elasticsearch:92 Search::Elasticsearch is instantiate with cxn_pool => 'Sniff' The doc says: """ The Sniff connection pool should be used when you do have direct access to the Elasticsearch cluster, eg when your web servers and Elasticsearch servers are on the same network. """ https://metacpan.org/pod/Search::Elasticsearch#cxn_pool We must make this option configurable to allow elastic cluster running on a different network. Original issue reporter on bug 21675 comment 8, they are connecting to the Amazon Elastic cluster.
Created attachment 83291 [details] [review] Bug 21974: Make Elasticsearch connection settings configurable Default settings are appended in get_elasticsearch_params so that there's a central place for them. Defaults can be overridden in koha-conf.xml. Test plan: 1. Verify that searches still work with the patch applied. 2. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml: <cxn_pool>Static</cxn_pool> 3. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml: <cxn_pool>FOO</cxn_pool>
Created attachment 86052 [details] [review] Bug 21974: Make Elasticsearch connection settings configurable Default settings are appended in get_elasticsearch_params so that there's a central place for them. Defaults can be overridden in koha-conf.xml. Test plan: 1. Verify that searches still work with the patch applied. 2. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml: <cxn_pool>Static</cxn_pool> 3. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml: <cxn_pool>FOO</cxn_pool> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
No tests here.. and we've snuck in configuration of not just cxn_pool, but also a couple of other variables. I'd also like to see these newly exposed variables documented somewhere.. Failing QA for now.
Documentation is one thing and I can take care of it. I'm not sure how to write any meaningful tests for this, however. Note that in practice the configurability changes nothing unless you actually add the settings to koha-conf.xml.
Interesting.. I can't find tests for this module at all.. only the sub-modules. Perhaps I'm missing something.. Either way.. lets see some minimal documentation added and we can get this moving again. Thanks Ere.
I've now added a bit of documentation: https://wiki.koha-community.org/wiki/Elasticsearch#Advanced_Connection_Settings
Created attachment 87006 [details] [review] Bug 21974: Make Elasticsearch connection settings configurable Default settings are appended in get_elasticsearch_params so that there's a central place for them. Defaults can be overridden in koha-conf.xml. Test plan: 1. Verify that searches still work with the patch applied. 2. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml: <cxn_pool>Static</cxn_pool> 3. Verify that search fails after adding the following under <elasticsearch> in koha-conf.xml: <cxn_pool>FOO</cxn_pool> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
OK, So I'm yielding a bit on the tests requirement here as there's not a precedent I can find for testing config checks. Thanks for the documentation and my apologies for not getting back to this sooner. Passed QA
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.05
not backporting ES bug fixes/features to 18.05.x
On bug 22705 there is a complain about the default value of cxn_pool.