It's a problem currently in D7 that people don't notice the warning in the Solr service class description to disable their processors, and subsequently have huge problems, e.g., when Tokenizer is enabled.
We should therefore add a way for service classes to influence the processor choice in some way.
Probably we should still let the user choose these processors, but just discourage them from doing it by some visual means. #2228737: Add an "Advanced mode" global setting could help a lot with that, allowing us to just completely hide the non-supported processors from non-expert users.
Estimated Value and Story Points
This issue was identified as a Beta Blocker for Drupal 8. We sat down and figured out the value proposition and amount of work (story points) for this issue.
Value and Story points are in the scale of fibonacci. Our minimum is 1, our maximum is 21. The higher, the more value or work a certain issue has.
Value : 8
Story Points: 5
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2228739-7--limit_processors_by_backend.patch | 5.17 KB | drunken monkey |
Comments
Comment #1
nick_vhComment #2
borisson_I'll write a test for this now, but this is a patch that implements the code already. I've tested this manually and that works.
Comment #4
borisson_Still need to get that test in, but this fixes the failures introduced in #2.
Comment #5
borisson_Added a test
Comment #6
borisson_This still applies, is there any work I can still do on this issue?
Comment #7
drunken monkeyOh, sorry for that! Seems I left the tab open to look at it later but then forgot. Thanks for bumping!
The patch looks very good, thanks! I guess we can very well leave it to the backend modules whether they want to provide a mechanism for other module defining processors to discourage them for that backend.
The patch was mostly great, I just don't really like the method name,
limitProcessorSuggestions. For me, that sounds a bit like it would return the compatible processors, not the discouraged ones, or maybe alter the array of processors. In the attached patch suggestgetDiscouragedProcessors()– also a bit unwieldy, but still an improvement in my opinion.I also don't think we need a new test backend class, we can just use the state (once again) to determine the method's return value in the existing test backend.
Please see my attached patch for the suggested changes and tell me what you think!
And, in any case, thanks again!
Comment #8
borisson_I like this. The new name is better.
Comment #9
drunken monkeyGreat to hear, thanks!
Committed.