Summary: | cxn_pool must be configurable | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Searching - Elasticsearch | Assignee: | Ere Maijala <ere.maijala> |
Status: | CLOSED FIXED | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | major | ||
Priority: | P5 - low | CC: | ere.maijala, josef.moravec, lucas, martin.renvoize, nick |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21675 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 22705, 26309 | ||
Attachments: |
Bug 21974: Make Elasticsearch connection settings configurable
Bug 21974: Make Elasticsearch connection settings configurable Bug 21974: Make Elasticsearch connection settings configurable |
Description
Jonathan Druart
2018-12-07 13:16:34 UTC
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. |