-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement preservable jobs and configurable jobs table name #11
Conversation
c4dee44
to
de5af05
Compare
…'s representation. It also helps if when you run tests locally, they actually test something.
… as well as MySQL
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
==========================================
+ Coverage 92.43% 98.47% +6.03%
==========================================
Files 7 7
Lines 238 262 +24
==========================================
+ Hits 220 258 +38
+ Misses 18 4 -14
|
…and ancient MySQL at the same time easier). Add testing of the "space" functionality. Improve test coverage to 98%.
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.
Changes look good, but I am immesurably bothered by sqlDb
(instead of sqlDB
) and databaseId
(instead of databaseID
), but I read the ship has sailed there melting 🫠
Yeah what in the Javascript is that about |
... Fine, fine... whining babies... 😂 |
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.
LGTM!
Preserved jobs, if enabled, are marked with a
completed
state in the database. Turning preservation off if it was previously on does not clear old jobs from the table; it just prevents new ones from being preserved.Configuring the jobs table name requires passing the same name (and optionally, space) to both the queue driver and the migration. This is necessary because there's no way for the migration to see the driver's configuration.
Closes #9.
Closes #10.