element-hq / element-hq/synapse
Schema delta `run_create` and `run_upgrade` are footguns
- Dominant language
- Python
- Stars
- 4.6k
- Forks
- 600
- Avg merge
- 5d 22h
- Merged PRs (30d)
- 51
Description
As https://element-hq.github.io/synapse/v1.158/development/database_schema.html#python-delta-files
```python
import synapse.config.homeserver
import synapse.storage.engines
import synapse.storage.types
def run_create(
cur: synapse.storage.types.Cursor,
database_engine: synapse.storage.engines.BaseDatabaseEngine,
) -> None:
"""Called whenever an existing or new database is to be upgraded"""
...
def run_upgrade(
cur: synapse.storage.types.Cursor,
database_engine: synapse.storage.engines.BaseDatabaseEngine,
config: synapse.config.homeserver.HomeServerConfig,
) -> None:
"""Called whenever an existing database is to be upgraded."""
...
```
These names are extremely confusing and it seems we have some deltas using the wrong one.
These should probably have been `run_create_or_upgrade` (or just `run`)
and `run_upgrade_only`.
Further, one schema delta refers to `run_update` (not a real function name) and it appears that the delta silently does nothing: https://github.com/matrix-org/synapse/pull/15817/files#diff-a093a368008812f42f6ace09b315c585a276f2018f52900118de81651c317cdaR19
Contributor guide
Research direction
Start with the Python delta-file documentation and the schema-delta code referenced by PR #15817. Trace how run_create, run_upgrade, and the incorrect run_update name are discovered and invoked for new and existing databases. Done means the function names and migration behavior are unambiguous, with the silently skipped delta covered by a regression test.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100