Just a suggestion, at the moment, but definitely worth discussing in my opinion:
D7 shows that keeping the backend's table information in the server configuration leads to various problems, especially combined with Features, and in D8 where "configuration" is actually a concept, not just a name, this is bound to get even worth.
I think (and I'm very open to input here) that the conceptually proper way to store this information would be in the new State system. If implemented properly, I think it would ensure that moving configuration between servers works properly, with tables correctly created/changed/dropped according to changed configuration, etc.
Implementation-wise I imagine we'd have to store in the state the "revision" to which the current database layout for an index conforms, to detect when a change needs to occur. (As the "revision" here, I'd be thinking about an MD5 hash of the fields information, or something like that.) But maybe it would be possible without that, too, just moving the data more or less verbatim from the server/backend configuration to the State system.
| Comment | File | Size | Author |
|---|---|---|---|
| #27 | 2564847-27--db_info_to_site_state.patch | 31.68 KB | drunken monkey |
| #26 | 2564847-26--db_info_to_site_state--interdiff.txt | 20.81 KB | drunken monkey |
| #26 | 2564847-26--db_info_to_site_state.patch | 27.44 KB | drunken monkey |
Comments
Comment #2
mollux commentedMakes sense to me to move that info to the state system. I don't think a md5 hash is needed, as the index name is unique.
The attached patch moves the info to the state system. Note that this will break existing sites, as there will be no info for the indexes for the database back-end anymore.
Comment #5
mollux commentedForgot to remove the config from the defaults db, should be fixed now.
Comment #6
mollux commentedComment #7
drunken monkeyThanks, great work!
Just a bit of refactoring to make the code look nicer, and the small fix (plus test) we discussed.
If the test bot is OK with this and you're fine with it, too, then this would be RTBC in my opinion.
Comment #9
drunken monkeyApparently, subkey-access for site state isn't a thing.
Comment #13
drunken monkeyComment #16
drunken monkeyComment #21
mollux commentedProbably fixed the failing tests
Comment #22
drunken monkeySorry, wrong issue!
Comment #23
drunken monkeyComment #24
drunken monkeyAs discussed, thanks a lot for finding this problem!
I also finally found Sascha (or, rather, he found me) and he also thinks this would be a great move.
However, I asked him about putting so much data into the site state, and he told me they'd probably add caching for it at some point so it would actually be better not to use the site state, but our own, separate key-value store directly. It's supposed to work almost the same, the state just provides a slightly simplified API to access key-value store functionality.
So one of us should probably re-write this to use that instead. Hopefully, it will just be straight-forward changes that won't break anything again.
Do you still have time to work on this, Mattias, or would you prefer if I took over? (Sorry I didn't ask Sascha earlier!)
Comment #25
drunken monkeyComment #26
drunken monkeyImplemented the switch. Let's hope for the best …
Comment #27
drunken monkeyRe-roll for latest commits, testing again just to make sure …
Comment #30
drunken monkeyAh, great, finally!
Committed.
Thanks again a lot for your work, Mattias!