timgit / pg-boss

Queueing jobs in Postgres from Node.js like a boss
MIT License
2.09k stars 158 forks source link

Feature Request: noContractor configuration #319

Closed MadhuVK closed 2 months ago

MadhuVK commented 2 years ago

Hey again!

I was wondering if we could add an option to disable the creation / migration of the pg-boss schema during startup.

Use-case: The DB connections used by our production pg-boss workers are not privileged to perform any DDL operations - we wish to perform any schema maintenance using our own migration scripts (taking advantage of the static functions listed under static functions). It would be nice if we could signal to pg-boss this expectation and send a clear error signal when upgrades are required in order to properly use the library.

The following is my rough proposal for introducing / leveraging such a config:

diff --git a/src/contractor.js b/src/contractor.js
index a77bad9..cb2679a 100644
--- a/src/contractor.js
+++ b/src/contractor.js
@@ -48,6 +48,10 @@ class Contractor {
   }

   async create () {
+    if (config.noContractor) {
+      throw new Error(`noContractor set - can't install pg-boss`)
+    }
+
     try {
       const commands = plans.create(this.config.schema, schemaVersion)
       await this.db.executeSql(commands)
@@ -57,6 +61,10 @@ class Contractor {
   }

   async migrate (version) {
+    if (config.noContractor) {
+      throw new Error(`noContractor set - can't migrate pg-boss`)
+    }
+
     try {
       const commands = migrationStore.migrate(this.config, version, this.migrations)
       await this.db.executeSql(commands)
@@ -66,11 +74,19 @@ class Contractor {
   }

   async next (version) {
+    if (config.noContractor) {
+      throw new Error(`noContractor set - can't migrate pg-boss`)
+    }
+
     const commands = migrationStore.next(this.config.schema, version, this.migrations)
     await this.db.executeSql(commands)
   }

   async rollback (version) {
+    if (config.noContractor) {
+      throw new Error(`noContractor set - can't rollback pg-boss`)
+    }
+
     const commands = migrationStore.rollback(this.config.schema, version, this.migrations)
     await this.db.executeSql(commands)
   }

Let me know what you think.

timgit commented 2 years ago

To make sure I'm understanding, you're requesting a friendly error message when a migration is required but not allowed?

dhermes commented 3 months ago

@timgit I am in a similar boat ("are not privileged to perform any DDL operations"), but it seems like the following could be a simpler approach:

diff --git a/src/index.js b/src/index.js
index 775bc34..806e4fb 100644
--- a/src/index.js
+++ b/src/index.js
@@ -98,7 +98,9 @@ class PgBoss extends EventEmitter {
       await this.db.open()
     }

-    await this.contractor.start()
+    if (!this.config.noContractor) {
+      await this.contractor.start()
+    }

     this.stopped = false
     this.started = true
diff --git a/types.d.ts b/types.d.ts
index 2ffab40..0eac1ed 100644
--- a/types.d.ts
+++ b/types.d.ts
@@ -49,6 +49,7 @@ declare namespace PgBoss {

   type ConstructorOptions =
     DatabaseOptions
+    & MigrationOptions
     & QueueOptions
     & SchedulingOptions
     & MaintenanceOptions
@@ -58,6 +59,10 @@ declare namespace PgBoss {
     & JobPollingOptions
     & CompletionOptions

+  interface MigrationOptions {
+    noContractor?: boolean;
+  }
+
   interface CompletionOptions {
     onComplete?: boolean;
   }

(Diff at f1c1636caf9518ec9cd3fe0d0e2844ee98179e68)

Separately, I'd be nice to expose some of the parts of the contractor on the PgBoss class, e.g. a version() method and potentially all of contractor.start() (likely with a different name, e.g. migrate())

timgit commented 3 months ago

The latest v10 beta has isInstalled() and version() added to the API

dhermes commented 3 months ago

@timgit Awesome, thanks!

What are your thoughts on noContractor (or same concept under a different name)?

timgit commented 3 months ago

I added migrate: false to the config options to block auto-migration if needed

dhermes commented 3 months ago

I added migrate: false to the config options to block auto-migration if needed

That's awesome thanks! I.e. it's in https://github.com/timgit/pg-boss/pull/425/files, which is not merged yet but released on npm as betas?

timgit commented 3 months ago

Yep!