refactoring: rewrite sync to class#137
Conversation
Single class for syncing Stripe entity. While the class itself is a bit large, it avoids having to do parameter drilling through many methods and actually reduces overall code by quite a bit. Also simplified backfills. This is prep work to eventually allow the sync engine to be it's own package.
⛔ Snyk checks have failed. 2 issues have been found so far.
⛔ code/snyk check is complete. 2 issues have been found. (View Details) 💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![snyk-io[bot]](https://avatars.githubusercontent.com/in/372950?s=60&v=4){kind=small}
This reverts commit 85143b7.
kevcodez
changed the title
kevcodez
changed the title
ecktoteckto
left a comment
ecktoteckto
left a comment
There was a problem hiding this comment.
-
Code looks good to me. Made a few changes/suggestions (mainly naming) and added one or two questions.
-
Tested successfully with
pnpm dev -
Running
docker run -p 8080:8080 stripe-sync-enginegives the following error.
node:internal/modules/cjs/loader:1404 throw err; ^ Error: Cannot find module '/app/pnpm' at Function._resolveFilename (node:internal/modules/cjs/loader:1401:15) at defaultResolveImpl (node:internal/modules/cjs/loader:1057:19) at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1062:22) at Function._load (node:internal/modules/cjs/loader:1211:37) at TracingChannel.traceSync (node:diagnostics_channel:322:14) at wrapModuleLoad (node:internal/modules/cjs/loader:235:24) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:171:5) at node:internal/main/run_main_module:36:49 { code: 'MODULE_NOT_FOUND', requireStack: [] }
| `select id from "${this.config.schema}"."customers" WHERE deleted <> true;` | ||
| )([]) | ||
|
|
||
| const customerIds = await this.postgresClient |
There was a problem hiding this comment.
Can that (further processing of the data) be problematic in case of a large number of customers?
There was a problem hiding this comment.
payment method sync is generally not recommended to run unless you know what you're doing given there is no proper pagination with creation date filter - this is the same as before though
Co-authored-by: Thomas <31189692+ecktoteckto@users.noreply.github.com>
kevcodez
merged commit 77068c5
into
kevin/billing-1390-disable-request-logging-and-log-events-in-stripe-sync-engine
Refactor to having a single class for syncing Stripe entities - while the class itself is a bit large, it avoids having to do parameter drilling through many methods and actually reduces overall code by quite a bit. Additionally - Reduced boilerplate for backfills (generic function) - Reduced boilerplate for deletions - Reduced boilerplate for auto-expanding lists - Migrated from jest to vitest - Sped up CI - Config option to enable/disable automatic backfills - Configurable max connections - Migrate to pnpm
Refactor to having a single class for syncing Stripe entities - while the class itself is a bit large, it avoids having to do parameter drilling through many methods and actually reduces overall code by quite a bit. Additionally - Reduced boilerplate for backfills (generic function) - Reduced boilerplate for deletions - Reduced boilerplate for auto-expanding lists - Migrated from jest to vitest - Sped up CI - Config option to enable/disable automatic backfills - Configurable max connections - Migrate to pnpm
* webhook filter * unit test
2 participants
Refactor to having a single class for syncing Stripe entities - while the class itself is a bit large, it avoids having to do parameter drilling through many methods and actually reduces overall code by quite a bit.
Additionally