When calling setBackendConfig, the backend plugin has no way to react to the configuration change unless it has been initialized.
#[ActionMethod(adminLabel: new TranslatableMarkup('Set backend config'), pluralize: FALSE)]
public function setBackendConfig(array $backend_config) {
$this->backend_config = $backend_config;
// In case the backend plugin is already loaded, make sure the configuration
// stays in sync.
if ($this->backendPlugin
&& $this->getBackend()->getConfiguration() !== $backend_config) {
$this->getBackend()->setConfiguration($backend_config);
}
return $this;
}
See #3516700: Determine host, context, and core from update_endpoint on setConfiguration as well using a recipe to set configuration for SearchStax using inputs and setBackendConfig
Always load the backend plugin and set its configuration
#[ActionMethod(adminLabel: new TranslatableMarkup('Set backend config'), pluralize: FALSE)]
public function setBackendConfig(array $backend_config) {
$this->getBackend()->setConfiguration($backend_config);
$this->backend_config = $this->getBackend()->getConfiguration();
return $this;
}
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #4
drunken monkeySure, that seems sensible. Thanks a lot for the suggestion!
I created an MR with your exact code. Feel free to give it a try. I’d also appreciate feedback from anyone else that wants to weigh in, otherwise I’d just merge it in a week or two.
There is a slight API change here in that
setBackendConfig()can now throw an exception, but I feel like that shouldn’t really matter in the real world. But I might still want to add a change record for this, just to be on the safe side – not sure yet.(Another option would be to catch the exception within
setBackendConfig(). Might also make sense, I guess, to just not callsetConfiguration()on the backend plugin if it can’t be loaded.)Comment #5
drunken monkeyChanged my mind, let’s not throw an exception after all. Doesn’t really add any value there, I’d say.
Comment #6
mglamanLooks good, and thanks! Makes sense to swallow up the exception for now. If other folks start doing more with recipes and find it useful it can be argued to fail loudly for DX around building recipes.
Thanks for making the MR, had to move on and didn't have time to open one.
Comment #7
drunken monkeyRunning this locally with XDebug enabled makes it immediately clear why PHPUnit fails to finish: The current code actually leads to an infinite loop, as the default method implementation
\Drupal\search_api\Backend\BackendPluginBase::setConfiguration()actually calls$this->server->setBackendConfig(). Our current code actually already guarded against this, so that the config stays in sync no matter which method you call but you should still never get an infinite loop.So, let’s just remove one condition from the
ifcheck and add thetry/catchand that should fix all that.Please test/review again!
Comment #8
thejimbirch commentedAdding tag so I can update the config action documentation when this lands.
Also asked Artem to review in the Drupal Slack.
Comment #9
a.dmitriiev commentedI have checked with the "real" recipe to use the config action to set the backend config and everything worked properly.
Comment #10
a.dmitriiev commentedComment #12
drunken monkeyGreat to hear, thanks for the feedback!
Merged.
Thanks again, everyone!