🔨 switch almost all remaining typeorm util function holdouts to knex #3359

danyx23 · 2024-03-16T13:34:39Z

This PR cleans up almost all remaining uses of TypeORM utility funcitons. The only remaining holdouts now are the ones in the chart revision tool related functions which will be done in a later PR.

danyx23 · 2024-03-16T13:34:54Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @danyx23 and the rest of your teammates on Graphite

ikesau

Nice! Really amazing effort on all of this - just a little bit more to go! 🏳️ 🏎️

ikesau · 2024-03-21T19:04:35Z

adminSiteServer/plainRouterHelpers.tsx

@@ -0,0 +1,105 @@
+import { NextFunction, Request, Response, Router } from "express"
+import * as db from "../db/db.js"
+export function getPlainRouteWithROTransaction<T>(


I wasn't even aware of the plain/functional router distinction!

Not for the PR of course, but do you think it's worth keeping both around?

Yeah I think we could clean that up but it doesn't seem particularly urgent

ikesau · 2024-03-21T19:08:38Z

adminSiteServer/testPageRouter.tsx

-testPageRouter.get("/embeds", async (req, res) => {
-    const props = await db.knexReadonlyTransaction((trx) =>
-        propsFromQueryParams(trx, {
+getPlainRouteWithROTransaction(


At http://localhost:3030/admin/test , only the explorer test embeds appear to work

Everything else gets:

TypeError: The operator "undefined" is not permitted

Ah nice, thanks for catching that. Fixed on the new top stack PR #3391

ikesau · 2024-03-21T19:16:12Z

baker/SiteBaker.tsx

@@ -431,8 +431,6 @@ export class SiteBaker {
    private async removeDeletedPosts(knex: db.KnexReadonlyTransaction) {
        if (!this.bakeSteps.has("removeDeletedPosts")) return

-        await db.getConnection()


I don't think this change has come from here, but I also checked in the head of this PR chain and it's happening there too so I'll just write it here that:

node itsJustJavascript/baker/buildLocalBake.js --steps specialPages

with

TypeError: Cannot read properties of null (reading 'toLocaleDateString') at formatDate (grapher/packages/@ourworldindata/utils/dist/Util.js:960:17) at grapher/itsJustJavascript/db/model/Gdoc/GdocBase.js:674:49 at Array.map (<anonymous>) at getMinimalGdocPostsByIds (grapher/itsJustJavascript/db/model/Gdoc/GdocBase.js:668:17) at process.processTicksAndRejections (node:internal/process/task_queues:95:5) at async GdocHomepage.loadLinkedDocuments (grapher/itsJustJavascript/db/model/Gdoc/GdocBase.js:507:33) at async GdocHomepage.loadState (grapher/itsJustJavascript/db/model/Gdoc/GdocBase.js:626:9) at async loadGdocFromGdocBase (grapher/itsJustJavascript/db/model/Gdoc/GdocFactory.js:199:5) at async renderFrontPage (grapher/itsJustJavascript/baker/siteRenderers.js:172:30) at async SiteBaker.bakeSpecialPages (grapher/itsJustJavascript/baker/SiteBaker.js:353:66)

ikesau · 2024-03-21T19:17:34Z

baker/batchTagWithGpt.ts

+// export const batchTagWithGpt = async ({
+//     debug,
+//     limit,
+// }: BatchTagWithGptArgs = {}) => {
+//     await db.knexReadonlyTransaction((trx) =>
+//         batchTagChartsWithGpt(trx, { debug, limit })
+//     )
+// }


Commented out?

ikesau · 2024-03-21T19:23:32Z

db/tests/basic.test.ts

@@ -1,18 +1,15 @@
 #! /usr/bin/env jest
 import sqlFixtures from "sql-fixtures"
 import { dbTestConfig } from "./dbTestConfig.js"
-import { dataSource } from "./dataSource.dbtests.js"


Nice! Hadn't remembered we had these - tested and confirmed working 👍

ikesau · 2024-03-21T19:31:37Z

adminSiteServer/adminRouter.tsx

-    if (!variableMetadata) throw new JsonError("No such variable", 404)
+getPlainRouteWithROTransaction(
+    adminRouter,
+    "/datapage-preview/:id",


I'm getting an error when trying to visit this route: http://localhost:3030/admin/datapage-preview/7216 which requires the server to be restarted

error: Error: Transaction query already complete, run with DEBUG=knex:tx for more info

Thanks for catching that, fixed also in the top PR

danyx23 · 2024-03-26T14:04:43Z

Merge activity

Mar 26, 10:04 AM EDT: @danyx23 started a stack merge that includes this pull request via Graphite.
Mar 26, 10:23 AM EDT: Graphite rebased this pull request as part of a merge.
Mar 26, 10:24 AM EDT: @danyx23 merged this pull request with Graphite.

The only remaining uses of the typeorm functions are related to the chart revision tool

This was referenced Mar 16, 2024

🚧 remove typeorm annotations from GDocs classes #3299

Merged

🔨 set up eslint rule to error on dangling promises + fix 245 violations #3358

Merged

github-actions bot assigned danyx23 Mar 16, 2024

danyx23 marked this pull request as ready for review March 16, 2024 13:39

danyx23 force-pushed the eslint-no-floating-promises branch from af5f9ca to b4aa5dc Compare March 16, 2024 13:46

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from ed42cc5 to ffac6a7 Compare March 16, 2024 13:47

danyx23 force-pushed the eslint-no-floating-promises branch from b4aa5dc to 5a640e2 Compare March 16, 2024 14:00

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from ffac6a7 to aa50f14 Compare March 16, 2024 14:00

danyx23 force-pushed the eslint-no-floating-promises branch from 5a640e2 to 0366771 Compare March 16, 2024 17:18

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from aa50f14 to 4de4a18 Compare March 16, 2024 17:18

danyx23 force-pushed the eslint-no-floating-promises branch from 0366771 to 34fca5b Compare March 16, 2024 17:31

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 4de4a18 to ea68c64 Compare March 16, 2024 17:31

danyx23 mentioned this pull request Mar 16, 2024

✨ unify transaction creation and close db connection in same call for scripts #3360

Merged

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from fe40423 to e559c80 Compare March 18, 2024 15:54

danyx23 force-pushed the eslint-no-floating-promises branch from f078be0 to 78bb8aa Compare March 18, 2024 18:59

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 94180e6 to 445e098 Compare March 18, 2024 18:59

danyx23 requested a review from ikesau March 18, 2024 19:00

danyx23 force-pushed the eslint-no-floating-promises branch from 78bb8aa to 29b9ac2 Compare March 19, 2024 16:02

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 445e098 to c968925 Compare March 19, 2024 16:02

danyx23 force-pushed the eslint-no-floating-promises branch from 29b9ac2 to b8f843c Compare March 20, 2024 10:05

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from c968925 to 79b18e1 Compare March 20, 2024 10:05

danyx23 mentioned this pull request Mar 20, 2024

🐛 Fix bug in SQL query #3372

Merged

danyx23 force-pushed the eslint-no-floating-promises branch from b8f843c to 849b260 Compare March 20, 2024 20:31

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 79b18e1 to 73526be Compare March 20, 2024 20:31

danyx23 mentioned this pull request Mar 20, 2024

🔨 images need a RW transaction in case they need to update the db 😢 - bubble this up #3385

Merged

ikesau requested changes Mar 21, 2024

View reviewed changes

danyx23 mentioned this pull request Mar 21, 2024

Fix remaining refactor issues #3391

Merged

danyx23 force-pushed the eslint-no-floating-promises branch from 849b260 to 9c83b2b Compare March 21, 2024 22:22

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 73526be to 1be5ef4 Compare March 21, 2024 22:22

danyx23 force-pushed the eslint-no-floating-promises branch from 9c83b2b to 9b21d17 Compare March 23, 2024 15:34

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 1be5ef4 to f8026fa Compare March 23, 2024 15:34

danyx23 mentioned this pull request Mar 23, 2024

🔨 refactor suggested chart revision tool to use knex #3396

Merged

danyx23 force-pushed the eslint-no-floating-promises branch from 9b21d17 to 049b0d1 Compare March 25, 2024 11:26

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from f8026fa to 65f7d9d Compare March 25, 2024 11:26

danyx23 force-pushed the eslint-no-floating-promises branch from 049b0d1 to f754031 Compare March 26, 2024 11:44

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 65f7d9d to 24084b8 Compare March 26, 2024 11:45

danyx23 force-pushed the eslint-no-floating-promises branch from f754031 to 95652fc Compare March 26, 2024 13:08

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 24084b8 to 299207d Compare March 26, 2024 13:08

danyx23 force-pushed the eslint-no-floating-promises branch from 95652fc to acbfb58 Compare March 26, 2024 14:19

Base automatically changed from eslint-no-floating-promises to master March 26, 2024 14:22

🔨 switch almost all remaining typeorm util function holdouts to knex

63d6d63

The only remaining uses of the typeorm functions are related to the chart revision tool

danyx23 force-pushed the db-cleanup-after-big-db-refactor branch from 299207d to 63d6d63 Compare March 26, 2024 14:23

danyx23 merged commit 53f4424 into master Mar 26, 2024
17 of 20 checks passed

danyx23 deleted the db-cleanup-after-big-db-refactor branch March 26, 2024 14:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🔨 switch almost all remaining typeorm util function holdouts to knex #3359

🔨 switch almost all remaining typeorm util function holdouts to knex #3359

danyx23 commented Mar 16, 2024 •

edited

Loading

danyx23 commented Mar 16, 2024 •

edited

Loading

ikesau left a comment

ikesau Mar 21, 2024

danyx23 Mar 21, 2024

ikesau Mar 21, 2024

danyx23 Mar 21, 2024

ikesau Mar 21, 2024

ikesau Mar 21, 2024

ikesau Mar 21, 2024

ikesau Mar 21, 2024

danyx23 Mar 21, 2024

danyx23 commented Mar 26, 2024 •

edited

Loading

🔨 switch almost all remaining typeorm util function holdouts to knex #3359

🔨 switch almost all remaining typeorm util function holdouts to knex #3359

Conversation

danyx23 commented Mar 16, 2024 • edited Loading

danyx23 commented Mar 16, 2024 • edited Loading

ikesau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danyx23 commented Mar 26, 2024 • edited Loading

Merge activity

danyx23 commented Mar 16, 2024 •

edited

Loading

danyx23 commented Mar 16, 2024 •

edited

Loading

danyx23 commented Mar 26, 2024 •

edited

Loading