Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Our config schema's are seriously broken sadly enough :(.
We have no schema for processors for example.
Because of that values are getting saved incorrectly in the yml file and a lot of core features rely on a correct schema.
Comment | File | Size | Author |
---|---|---|---|
#13 | interdiff.txt | 975 bytes | amateescu |
#13 | 2235381-13.patch | 10.26 KB | amateescu |
Comments
Comment #1
drunken monkeyChanging the title to reflect the larger scope. Missing are, as far as I can see:
search_api.datasource.plugin.[%parent.datasourcePluginId]
magic does, unless I'm mistaken.Also, though that's maybe a different issue, the properties are rather inconsistent.
machine_name
vs.serverMachineName
, for example, anddatasourcePluginId
is just a bit verbose. Same for the option names:cron_limit
vs.additional fields
.Comment #2
drunken monkeyComment #3
amateescu CreditAttribution: amateescu commentedAbout "machine_name vs. serverMachineName", core tries to follow the snake case pattern for this (but doesn't always succeed :P), we should try to match that as well.
And "additional fields" is definitely wrong, it should have an underscore there.
I'm wondering if this issue should wait for any other architectural changes, like #2230931: Supporting multiple item types per index. By the way, are there any other big ones besides that?
Comment #4
drunken monkey#2241429: Support translations might also be a larger change, but should ideally only affect the content entity datasource.
Other than that, #2044421: [meta] Upgrade to Drupal 8 lists all the current goals for the module. I don't think there are any larger architectural changes there, but until RC status, anything could still come up.
In any case, I think we can just do this at any point at which we want to, and just (try to) remember afterwards to also update the config schema when making any related changes.
Comment #5
amateescu CreditAttribution: amateescu commentedCommitted a small patch for search_api.settings.yml: http://drupalcode.org/sandbox/daeron/2091893.git/commitdiff/60dbcd2
I think the rest should be postponed on #2230931: Supporting multiple item types per index so we don't cause unnecessary conflicts.
Comment #6
drunken monkeyAgreed. Thanks!
Comment #7
drunken monkeyHm, I think this can be un-postponed now?
Does anyone want to work on this? Or should we still wait, until we are moving back to the original repository?
Comment #8
aspilicious CreditAttribution: aspilicious commentedI took a day idea of the recover from this weekend. I think I can handle this patch today.
Comment #9
drunken monkeyThanks a lot!
Sorry, I was busy with other work before so couldn't respond in IRC:
It was like that before, and I would like it much better, too, but someone said for the config schema having it in two separate properties makes more sense. If this is not the case, all the better!
In general, do what you must to make this work, the current storage schema isn't set in stone. I'd also be far from against overriding
toArray()
– at least we could then finally declare$original
as a property and get rid of a few code style warnings. ;)Comment #11
drunken monkeyFrom short dicussion with Nick:
search_api.processor.plugin.node_status
seems to lack a mapping for its settings.Comment #12
amateescu CreditAttribution: amateescu commentedThis patch should fix all the issues that I see with the current schemas. Note that the "datasources + datasources_configs" problem is not finished yet but pending on the resolution of #2291073: Config schema doesn't support referencing plugin derivatives in core.
Comment #13
amateescu CreditAttribution: amateescu commentedFixed a wrong indentation, noticed by @aspilicious in IRC.
Comment #14
aspilicious CreditAttribution: aspilicious commentedGood to go! But we need an issue to track an issue about empty configuration.
==> search_api.tracker.schema.yml
No idea how to handle this, maybe removing is ok? Maybe we should open a core issue?
Comment #15
amateescu CreditAttribution: amateescu commentedI'm pretty sure removing the schema for plugins that don't have any configuration is perfectly ok.