-
Notifications
You must be signed in to change notification settings - Fork 77
feat(notifications): upgrades pg-boss #2171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughRemoved provider-level Changes
Sequence Diagram(s)sequenceDiagram
participant BrokerSvc as BrokerService
participant Config as ConfigService
participant PgBoss as PgBossProvider
Note over PgBoss: Provider initialized without\narchiveCompletedAfterSeconds (pg-boss v11)
BrokerSvc->>Config: get(broker.EVENT_BROKER_ARCHIVE_COMPLETED_AFTER_SECONDS)
Config-->>BrokerSvc: deleteAfterSeconds
BrokerSvc->>PgBoss: createQueue(queueName, { deleteAfterSeconds })
PgBoss-->>BrokerSvc: queue created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (3)**/*.spec.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
🔇 Additional comments (2)
Comment |
| "dev": true, | ||
| "license": "ISC", | ||
| "bin": { | ||
| "semver": "bin/semver" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔄 Carefully review the package-lock.json diff
Resolve the comment if everything is ok
* node_modules/@asamuzakjp/css-color 3.2.0 -> 3.1.4
* node_modules/@babel/compat-data 7.28.5 -> 7.24.7
* node_modules/@babel/core 7.28.5 -> 7.24.7
* node_modules/@babel/generator 7.28.5 -> 7.26.2
* node_modules/@babel/helper-compilation-targets 7.27.2 -> 7.24.7
* node_modules/@babel/helper-module-imports 7.27.1 -> 7.25.9
* node_modules/@babel/helper-module-transforms 7.28.3 -> 7.26.0
* node_modules/@babel/helper-plugin-utils 7.27.1 -> 7.25.9
* node_modules/@babel/helper-validator-identifier 7.28.5 -> 7.27.1
* node_modules/@babel/helper-validator-option 7.27.1 -> 7.25.9
* node_modules/@babel/helpers 7.28.4 -> 7.27.6
* node_modules/@babel/parser 7.28.5 -> 7.27.2
* node_modules/@babel/plugin-syntax-jsx 7.27.1 -> 7.25.9
* node_modules/@babel/plugin-syntax-typescript 7.27.1 -> 7.25.9
* node_modules/@babel/traverse 7.28.5 -> 7.25.9
* node_modules/@babel/types 7.28.5 -> 7.27.6
* node_modules/@jridgewell/gen-mapping 0.3.13 -> 0.3.5
* node_modules/@jridgewell/trace-mapping 0.3.31 -> 0.3.25
* node_modules/@pkgr/core 0.2.9 -> 0.2.0
* node_modules/@types/yargs 17.0.34 -> 17.0.32
* node_modules/@ungap/structured-clone 1.3.0 -> 1.2.0
* node_modules/babel-preset-current-node-syntax 1.2.0 -> 1.0.1
* node_modules/cron-parser 4.9.0 -> 5.4.0
* node_modules/dedent 1.7.0 -> 1.5.3
* node_modules/import-local 3.2.0 -> 3.1.0
* node_modules/luxon 3.5.0 -> 3.7.2
* node_modules/pg-boss 10.3.2 -> 11.1.2
* node_modules/pirates 4.0.7 -> 4.0.6
+ node_modules/@ampproject/remapping 2.3.0
+ node_modules/@jridgewell/set-array 1.2.1
- apps/api/node_modules/pg-boss 11.0.4
- apps/deploy-web/node_modules/@jest/console 30.2.0
- apps/deploy-web/node_modules/@jest/core 30.2.0
- apps/deploy-web/node_modules/@jest/expect-utils 30.2.0
- apps/deploy-web/node_modules/@jest/expect 30.2.0
- apps/deploy-web/node_modules/@jest/globals 30.2.0
- apps/deploy-web/node_modules/@jest/reporters 30.2.0
- apps/deploy-web/node_modules/@jest/source-map 30.0.1
- apps/deploy-web/node_modules/@jest/test-result 30.2.0
- apps/deploy-web/node_modules/@jest/test-sequencer 30.2.0
- apps/deploy-web/node_modules/@jest/transform 30.2.0
- apps/deploy-web/node_modules/agent-base 7.1.4
- apps/deploy-web/node_modules/babel-jest 30.2.0
- apps/deploy-web/node_modules/babel-plugin-istanbul 7.0.1
- apps/deploy-web/node_modules/babel-plugin-jest-hoist 30.2.0
- apps/deploy-web/node_modules/babel-preset-jest 30.2.0
- apps/deploy-web/node_modules/camelcase 6.3.0
- apps/deploy-web/node_modules/cjs-module-lexer 2.1.1
- apps/deploy-web/node_modules/convert-source-map 2.0.0
- apps/deploy-web/node_modules/cssstyle 4.6.0
- apps/deploy-web/node_modules/data-urls 5.0.0
- apps/deploy-web/node_modules/expect 30.2.0
- apps/deploy-web/node_modules/html-encoding-sniffer 4.0.0
- apps/deploy-web/node_modules/iconv-lite 0.6.3
- apps/deploy-web/node_modules/istanbul-lib-source-maps 5.0.6
- apps/deploy-web/node_modules/jackspeak 3.4.3
- apps/deploy-web/node_modules/jest-changed-files 30.2.0
- apps/deploy-web/node_modules/jest-circus 30.2.0
- apps/deploy-web/node_modules/jest-cli/node_modules/jest-config 30.2.0
- apps/deploy-web/node_modules/jest-cli 30.2.0
- apps/deploy-web/node_modules/jest-diff 30.2.0
- apps/deploy-web/node_modules/jest-docblock 30.2.0
- apps/deploy-web/node_modules/jest-each 30.2.0
- apps/deploy-web/node_modules/jest-environment-jsdom 30.2.0
- apps/deploy-web/node_modules/jest-environment-node 30.2.0
- apps/deploy-web/node_modules/jest-haste-map 30.2.0
- apps/deploy-web/node_modules/jest-leak-detector 30.2.0
- apps/deploy-web/node_modules/jest-matcher-utils 30.2.0
- apps/deploy-web/node_modules/jest-resolve-dependencies 30.2.0
- apps/deploy-web/node_modules/jest-resolve 30.2.0
- apps/deploy-web/node_modules/jest-runner 30.2.0
- apps/deploy-web/node_modules/jest-runtime/node_modules/glob 10.4.5
- apps/deploy-web/node_modules/jest-runtime/node_modules/minipass 7.1.2
- apps/deploy-web/node_modules/jest-runtime 30.2.0
- apps/deploy-web/node_modules/jest-snapshot 30.2.0
- apps/deploy-web/node_modules/jest-validate 30.2.0
- apps/deploy-web/node_modules/jest-watcher/node_modules/chalk 4.1.2
- apps/deploy-web/node_modules/jest-watcher 30.2.0
- apps/deploy-web/node_modules/jest-worker/node_modules/supports-color 8.1.1
- apps/deploy-web/node_modules/jest-worker 30.2.0
- apps/deploy-web/node_modules/jest 30.2.0
- apps/deploy-web/node_modules/jsdom/node_modules/https-proxy-agent 7.0.6
- apps/deploy-web/node_modules/jsdom/node_modules/ws 8.18.3
- apps/deploy-web/node_modules/jsdom 26.1.0
- apps/deploy-web/node_modules/pure-rand 7.0.1
- apps/deploy-web/node_modules/semver 7.7.3
- apps/deploy-web/node_modules/signal-exit 4.1.0
- apps/deploy-web/node_modules/tough-cookie 5.1.2
- apps/deploy-web/node_modules/tr46 5.1.1
- apps/deploy-web/node_modules/w3c-xmlserializer 5.0.0
- apps/deploy-web/node_modules/webidl-conversions 7.0.0
- apps/deploy-web/node_modules/whatwg-encoding 3.1.1
- apps/deploy-web/node_modules/whatwg-mimetype 4.0.0
- apps/deploy-web/node_modules/whatwg-url 14.2.0
- apps/deploy-web/node_modules/write-file-atomic 5.0.1
- apps/deploy-web/node_modules/xml-name-validator 5.0.0
- node_modules/@babel/helper-globals 7.28.0
- node_modules/@jest/diff-sequences 30.0.1
- node_modules/@jest/environment-jsdom-abstract/node_modules/@jest/environment 30.2.0
- node_modules/@jest/environment-jsdom-abstract/node_modules/@jest/fake-timers 30.2.0
- node_modules/@jest/environment-jsdom-abstract/node_modules/@sinonjs/fake-timers 13.0.5
- node_modules/@jest/environment-jsdom-abstract/node_modules/@types/jsdom 21.1.7
- node_modules/@jest/environment-jsdom-abstract/node_modules/ansi-styles 5.2.0
- node_modules/@jest/environment-jsdom-abstract/node_modules/ci-info 4.3.1
- node_modules/@jest/environment-jsdom-abstract/node_modules/jest-message-util 30.2.0
- node_modules/@jest/environment-jsdom-abstract/node_modules/jest-mock 30.2.0
- node_modules/@jest/environment-jsdom-abstract/node_modules/jest-util 30.2.0
- node_modules/@jest/environment-jsdom-abstract/node_modules/picomatch 4.0.3
- node_modules/@jest/environment-jsdom-abstract/node_modules/pretty-format 30.2.0
- node_modules/@jest/environment-jsdom-abstract 30.2.0
- node_modules/@jest/get-type 30.1.0
- node_modules/@jest/pattern/node_modules/jest-regex-util 30.0.1
- node_modules/@jest/pattern 30.0.1
- node_modules/@jest/snapshot-utils/node_modules/@jest/schemas 30.0.5
- node_modules/@jest/snapshot-utils/node_modules/@jest/types 30.2.0
- node_modules/@jest/snapshot-utils/node_modules/@sinclair/typebox 0.34.41
- node_modules/@jest/snapshot-utils 30.2.0
- node_modules/@jridgewell/remapping 2.3.5
- node_modules/exit-x 0.2.2
- node_modules/synckit 0.11.11
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
c1f4325 to
38d4982
Compare
a0c75ab to
e15369b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts (1)
17-23: Fix TypeScript type safety violations in executeSql function.The queue-level
deleteAfterSecondsconfiguration is properly set in broker.service.ts (line 40) using the same config key. However, the executeSql function at lines 19-20 usesanytypes, violating the coding guidelines. Replaceany[]andanywith proper types:executeSql(text: string, values: unknown[]): Promise<{ rows: unknown[] }> { return client.query(text, values); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
apps/notifications/package.json(1 hunks)apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.spec.ts(0 hunks)apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts(1 hunks)apps/notifications/src/infrastructure/broker/services/broker/broker.service.spec.ts(1 hunks)apps/notifications/src/infrastructure/broker/services/broker/broker.service.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/notifications/src/infrastructure/broker/services/broker/broker.service.ts
- apps/notifications/src/infrastructure/broker/services/broker/broker.service.spec.ts
- apps/notifications/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: log-collector-ci
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate (apps/provider-console) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts
Show resolved
Hide resolved
40c6a83 to
bf25677
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts (1)
19-20: Replaceanytypes with proper TypeScript types.The
executeSqlfunction still usesanyfor both thevaluesparameter and therowsreturn type, which violates the coding guidelines that explicitly state: "Never use type any or cast to type any."As per coding guidelines.
Apply this diff to use proper TypeScript types:
db: { - executeSql(text: string, values: any[]): Promise<{ rows: any[] }> { + executeSql<T = Record<string, unknown>>(text: string, values?: unknown[]): Promise<{ rows: T[] }> { return client.query(text, values); } }Alternatively, import
QueryResultfrompgfor more precise typing:import { Client, QueryResult } from "pg";Then use:
db: { - executeSql(text: string, values: any[]): Promise<{ rows: any[] }> { + executeSql<T = Record<string, unknown>>(text: string, values?: unknown[]): Promise<QueryResult<T>> { return client.query(text, values); } }
🧹 Nitpick comments (1)
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts (1)
12-15: Consider investigating the custom db migration issue.The TODO comment indicates an unresolved issue where the custom
dbwrapper cannot be used for migrations. With the pg-boss v11 upgrade, this might be a good opportunity to revisit this limitation and determine if it's still necessary or if the new version handles this differently.Would you like me to help investigate why the custom db wrapper fails for migrations, or should I open an issue to track this investigation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
apps/api/src/core/services/job-queue/job-queue.service.ts(0 hunks)apps/notifications/package.json(1 hunks)apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.spec.ts(0 hunks)apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts(1 hunks)apps/notifications/src/infrastructure/broker/services/broker/broker.service.spec.ts(1 hunks)apps/notifications/src/infrastructure/broker/services/broker/broker.service.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.spec.ts
- apps/api/src/core/services/job-queue/job-queue.service.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/notifications/src/infrastructure/broker/services/broker/broker.service.spec.ts
- apps/notifications/src/infrastructure/broker/services/broker/broker.service.ts
- apps/notifications/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts
🧠 Learnings (1)
📚 Learning: 2025-07-27T12:16:08.566Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-07-27T12:16:08.566Z
Learning: Applies to **/*.{ts,tsx} : Never use type any or cast to type any. Always define the proper TypeScript types.
Applied to files:
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: codecov/project/notifications
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (1)
apps/notifications/src/infrastructure/broker/providers/pg-boss/pg-boss.provider.ts (1)
17-23: Now I need to verify the actual file content to ensure the review comment is based on the correct state:
The removal of
archiveCompletedAfterSecondsis correct and properly implemented. The pg-boss Db interface itself requires theexecuteSqlmethod withany[]for the values parameter, so implementing this custom db wrapper with the necessaryanytypes is not a code violation—it's fulfilling the library's interface contract. The queue-level retention configuration viadeleteAfterSecondsincreateQueuecalls is the proper pg-boss v11+ pattern and is correctly implemented in the codebase.Likely an incorrect or invalid review comment.
bf25677 to
ad213cc
Compare
| const queueName = this.toQueueName(eventName); | ||
| await this.boss.createQueue(queueName); | ||
| await this.boss.createQueue(queueName, { | ||
| deleteAfterSeconds: this.configService.getOrThrow("broker.EVENT_BROKER_ARCHIVE_COMPLETED_AFTER_SECONDS") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought(non-blocking): one important difference in v1 is that pg-boss no longer partition jobs table by default and if you need to enable this behavior, it should be explicit. https://timgit.github.io/pg-boss/#/./api/queues
refs #1779
Summary by CodeRabbit
Chores
Bug Fixes
Tests