Load configuration settings from the new "common" section. #727

Closed
wants to merge 2 commits into
from

Projects

None yet

2 participants

@trevorsmith

A new "common" section was added to the Sphinx configuration in 2.2.1.
There is a corresponding change to the Riddle repository.

See http://sphinxsearch.com/docs/manual-2.2.1.html#confgroup-common for
more information.

@trevorsmith trevorsmith Load configuration settings from the new "common" section.
A new "common" section was added to the Sphinx configuration in 2.2.1.
There is a corresponding change to the Riddle repository.

See http://sphinxsearch.com/docs/manual-2.2.1.html#confgroup-common for
more information.
33a76b9
Owner
pat commented Feb 14, 2014

Hi Trevor

Appreciate these commits (here and in Riddle)... but just thinking - if someone uses one of these settings when they're not using Sphinx 2.2.x, they're going to hit some errors. Some of the underlying settings existed before 2.2.1, otherwise it would have been fine.

Right now, I'm not sure of the best way to deal with this. I'm not keen on detecting Sphinx versions (like Riddle does and TS used to), because the added complexity/support issues is not worth the simplicity. Perhaps only use the common section if common: true is in thinking_sphinx.yml, like the utf8 setting in 3.0.6/3.1.0? And then at some point it becomes the default (and has to be set to false to be avoided).

(Apologies for the delay, I've been on vacation.)

I should've researched and tested with older versions of Sphinx. I'll be sure to do that for future contributions.

Your common: true setting is a simple and easy solution. As with the utf8 option, though, you might have a lot of developers who don't see the docs and submit issues or take up your time on the mailing list.

You might consider enabling the common section if lemmatizer_base is set. The other options – for the time being – can be set in either section without any complaints from Sphinx. If other common settings are added in future betas, we could detect them, too.

A combination of both strategies might work best; devs can manually set common: true if they are so inclined, and Thinking Sphinx can force it to be true if any 2.2.x options are set. Thoughts?

Owner
pat commented Feb 23, 2014

I think I'd rather keep it simpler - just the common: true (or, perhaps to make it a little less ambigious, common_sphinx_configuration: true), and have it default to false right now (and then when TS supports 2.2.x by default, it can be true by default).

I'm okay with a few more support questions about this (given most devs won't use these settings), instead of more complex code.

@trevorsmith trevorsmith Check for configuration accessors instead of the settings array.
This change enables the common_sphinx_configuration setting to be set in Riddle's Indexer and Common sections without the key being present (and rendered) to the final configuration file.
886528a

I spent a few minutes implementing the common_sphinx_configuration setting in riddle (pat/riddle#83).

@pat pat added the feature label Apr 8, 2014
Owner
pat commented Apr 19, 2014

Implemented this in 8da090c - thanks for the discussion and pull requests :)

@pat pat closed this Apr 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment