-
Notifications
You must be signed in to change notification settings - Fork 207
Closed
Labels
backendRequires a change to the API serverRequires a change to the API serverrefactorImproves code without altering behaviorImproves code without altering behavior
Description
This isn't actively a bug, but seems a likely place for future surprises:
It appears that there are issues with promise handling at https://github.com/getodk/central-backend/blob/f4e501de8ae35679ee5376a864649ca60a17462f/lib/model/migrations/20250927-01-geoextracts.js#L36-L41:
const up = async (db) => {
getSqlFiles('up').forEach(async sql => {
// See sidecar .sql files
await db.raw(sql);
});
};- what order should the
.sqlfiles be executed in? - should the migrator wait for this migration to complete before moving onto the next migration?
In fact, ordering is guaranteed because of a combination of knex & postgres behaviour.
Improvements
If ordering of individual migrations is not required, a possible code change to communicate this could be:
const up = async (db) => {
await Promise.all([
getSqlFiles('up').map(async sql => {
// See sidecar .sql files
await db.raw(sql);
});
]);
};If ordering of individual migrations is required, a possible code change to communicate this could be:
const up = async (db) => {
for(const sql of getSqlFiles('up')) {
// See sidecar .sql files
await db.raw(sql);
}
};Metadata
Metadata
Assignees
Labels
backendRequires a change to the API serverRequires a change to the API serverrefactorImproves code without altering behaviorImproves code without altering behavior
Type
Projects
Status
✅ done