Skip to content

fix: min concurrency #28

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 30, 2025
Merged

fix: min concurrency #28

merged 3 commits into from
Jul 30, 2025

Conversation

merencia
Copy link
Contributor

@merencia merencia commented Jul 30, 2025 •
edited
Loading

Checklist for Pull Requests

  • All tests pass (yarn test:all)
  • Code follows the style guide and passes lint checks
  • Documentation is updated (README, docs, etc)
  • Linked to corresponding issue, if applicable

Summary of Changes

Describe the changes made in this pull request. Include any relevant context or motivation for the changes.

@merencia merencia requested a review from GiovaniGuizzo as a code owner July 30, 2025 02:45
@merencia merencia force-pushed the fix/min_concurrency branch from dfa90cc to 919606e Compare July 30, 2025 02:47
@merencia merencia requested a review from Copilot July 30, 2025 02:51
Copilot

This comment was marked as outdated.

Sign in to view

GiovaniGuizzo
GiovaniGuizzo reviewed Jul 30, 2025
View reviewed changes
@merencia merencia requested a review from Copilot July 30, 2025 14:26
Copilot
Copilot AI reviewed Jul 30, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR changes the minimum concurrency validation from allowing 0 (unlimited) to requiring at least 1 for all queue operations. The change enforces that concurrency must be at least 1 across all backend implementations and configuration methods.

  • Removed validation allowing concurrency of 0 from queue operations
  • Added minimum concurrency validation (≥1) to all backend implementations
  • Updated tests to use valid concurrency values and added validation tests

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/sidequest/src/operations/queue.ts Removed validation allowing concurrency ≥ 0
packages/sidequest/src/operations/queue.test.ts Removed test for negative concurrency validation
packages/engine/src/queue/grant-queue-config.test.ts Added test for minimum concurrency validation
packages/engine/src/execution/executor-manager.test.ts Updated tests to use valid concurrency values with mocking
packages/engine/src/execution/dispatcher.test.ts Updated test to use mocking instead of 0 concurrency
packages/engine/src/engine.ts Added validation for maxConcurrentJobs minimum value
packages/engine/src/engine.test.ts Added test for maxConcurrentJobs validation
packages/backends/mysql/src/mysql-backend.ts Added minimum concurrency validation to create/update methods
packages/backends/mongo/src/mongo-backend.ts Added minimum concurrency validation to create/update methods
packages/backends/backend/src/sql-backend.ts Added minimum concurrency validation to create/update methods
packages/backends/backend-test/src/updateQueue.ts Added test for minimum concurrency validation
packages/backends/backend-test/src/createNewQueue.ts Added test for minimum concurrency validation
Comments suppressed due to low confidence (3)

packages/engine/src/execution/executor-manager.test.ts:228

  • This test adds an extra execute call but doesn't verify its behavior or wait for completion. Consider removing this line or adding proper verification of the intended test scenario.
      void executorManager.execute(queryConfig, jobData);

packages/engine/src/execution/executor-manager.test.ts:246

  • This test adds an extra execute call but doesn't verify its behavior or wait for completion. Consider removing this line or adding proper verification of the intended test scenario.
      void executorManager.execute(queryConfig, jobData);

packages/engine/src/execution/dispatcher.test.ts:87

  • The test expects claimPendingJob to not be called, but there's no guarantee the dispatcher has had time to attempt claiming. Consider adding a timeout or using waitUntil to ensure the test assertion is meaningful.
      expect(mockClaim).not.toBeCalled();

@merencia merencia merged commit 5e9877b into develop Jul 30, 2025
2 checks passed
@merencia merencia deleted the fix/min_concurrency branch July 30, 2025 14:44
sidequest-release bot pushed a commit that referenced this pull request Jul 30, 2025
@semantic-release-bot
chore(release): 1.0.0-next.17 [skip ci]
a2af8f7 
# [1.0.0-next.17](v1.0.0-next.16...v1.0.0-next.17) (2025-07-30)

### Bug Fixes

* min concurrency ([#28](#28)) ([5e9877b](5e9877b))
@sidequest-release
Copy link

🎉 This PR is included in version 1.0.0-next.17 🎉

The release is available on:

Your semantic-release bot 📦🚀

sidequest-release bot pushed a commit that referenced this pull request Aug 1, 2025
@semantic-release-bot
chore(release): 1.0.0 [skip ci]
00f3ba9 
# 1.0.0 (2025-08-01)

### Bug Fixes

* add close method to Engine class for backend cleanup ([90ece50](90ece50))
* add engines field to specify required Node.js version ([#42](#42)) ([44c9523](44c9523))
* author ([ec1734a](ec1734a))
* author ([e81372f](e81372f))
* changed to patch and fixed job path call on table ([6e2399e](6e2399e))
* claim ([b535e98](b535e98))
* deduplication ([#58](#58)) ([e0d6ae7](e0d6ae7))
* default job state ([626882e](626882e))
* enqueue ([a1cc0aa](a1cc0aa))
* ensure consistent path formatting in buildPath function ([55de875](55de875))
* escape backslashes in file paths for consistency ([36cc48c](36cc48c))
* fix build of dashboard on windows ([e92f15d](e92f15d))
* Fix Dashboard stats and graph, and removes Redis ([#154](https://github.com/sidequestjs/sidequest/issues/154)) ([b3ab912](b3ab912))
* fix second shutdown attempt ([295f6b4](295f6b4))
* fixed a few inconsistencies ([54d338d](54d338d))
* fixed time range ([fb19cac](fb19cac))
* fixed yarn lock ([#140](https://github.com/sidequestjs/sidequest/issues/140)) ([68176ec](68176ec))
* format ([c51ecf6](c51ecf6))
* Future canceled jobs not running ([#34](#34)) ([85a5212](85a5212))
* improve job failure handling in RetryTransition ([#143](https://github.com/sidequestjs/sidequest/issues/143)) ([f448c35](f448c35))
* including migrations on backends ([a670e89](a670e89))
* make config parameter optional in Engine.start method ([2359040](2359040))
* min concurrency ([#28](#28)) ([5e9877b](5e9877b))
* new version ([c104052](c104052))
* Proper dashboard and Sidequest shutdown ([#16](#16)) ([ed1d853](ed1d853))
* publish ([17d036b](17d036b))
* publish to npm ([477e9a2](477e9a2))
* queue list ([b09d103](b09d103))
* release ([4b2f4e7](4b2f4e7))
* release ([d731dd2](d731dd2))
* release ([b736f50](b736f50))
* release ([#26](#26)) ([c6093ae](c6093ae))
* release next ([66b9329](66b9329))
* release next ([dd9f2af](dd9f2af))
* release next ([cc4b2bd](cc4b2bd))
* release next ([586ec83](586ec83))
* release.yml ([c37c57c](c37c57c))
* remove topological ([f25b881](f25b881))
* remove unused delete button from job view ([#36](#36)) ([eedd15f](eedd15f))
* removing returning usage from sql backend ([5d61c29](5d61c29))
* removing version from monorepo ([e4c5a76](e4c5a76))
* removing version script ([e7736a4](e7736a4))
* revert unlimited jobs ([#23](#23)) ([96ccc5d](96ccc5d)), closes [#21](#21)
* sending config to sidequest main ([44a314c](44a314c))
* serialize errors ([#44](#44)) ([683b7e1](683b7e1))
* show more and show less not saving state on page refresh ([#41](#41)) ([fcc99e7](fcc99e7))
* states ([81be666](81be666))
* test ([1da410e](1da410e))
* test ([8444aef](8444aef))
* Unhandled error on Executor Manager ([#24](#24)) ([81bd5e0](81bd5e0))
* update default concurrency value in documentation ([dfc328d](dfc328d))
* update jobDefaults handling and add engine tests ([#4](#4)) ([bbd8dde](bbd8dde))
* update logger import to use @sidequest/core ([d1f7825](d1f7825))
* update staleJobs test to handle negative timeout values ([514b330](514b330))
* url ([74651c2](74651c2))
* using exec ([80e47be](80e47be))
* using personal token ([#25](#25)) ([91c5834](91c5834))
* using sidequest release app ([1ecf1a7](1ecf1a7))
* version ([be0e7d4](be0e7d4))
* versions ([903c601](903c601))
* versions ([abd5a80](abd5a80))
* yarn lock ([d9cdc68](d9cdc68))

### Features

* `Sidequest.build` from job ([#20](#20)) ([d7f5286](d7f5286))
* add better logging and tests for createBackendFromDriver function ([#142](https://github.com/sidequestjs/sidequest/issues/142)) ([00942cb](00942cb))
* add comprehensive test suite for job and queue management ([a775de8](a775de8))
* add job builder ([#39](#39)) ([5af07bb](5af07bb))
* add JobData to job object. ([#171](https://github.com/sidequestjs/sidequest/issues/171)) ([c61df01](c61df01))
* add nullable timeout column to sidequest_jobs table ([b2bacbd](b2bacbd))
* add timeout on executor start ([5c0164d](5c0164d))
* add updateJob method and related tests for job management ([6f7a024](6f7a024))
* added cli ([442b031](442b031))
* added dasy ui ([59411e9](59411e9))
* added job view - work in progress ([e274af6](e274af6))
* added mongodb backend ([#155](https://github.com/sidequestjs/sidequest/issues/155)) ([0149d82](0149d82))
* allow nested jobs ([#55](#55)) ([a8ff7f0](a8ff7f0))
* allowing to set maxAttempts and availabeAt on enqueuing job. ([#77](#77)) ([c469da2](c469da2))
* basic auth on dashboard ([#66](#66)) ([add020b](add020b))
* cancel job ([8434c70](8434c70))
* cleanup job and coverage ([#76](#76)) ([f58f2d0](f58f2d0))
* dashboard config ([e6741d2](e6741d2))
* enhance queue management with defaults and force update options ([#5](#5)) ([40acbec](40acbec))
* Facades and better transitions ([#161](https://github.com/sidequestjs/sidequest/issues/161)) ([8066f5d](8066f5d))
* implement backend initialization and configuration for dashboard ([34737b9](34737b9))
* implement staleJobs method for job management across backends ([a10959a](a10959a))
* improve DX ([#71](#71)) ([075b589](075b589))
* job flow control ([#42](#42)) ([f7dd600](f7dd600))
* job list ([5208e7c](5208e7c))
* job view ([31f6112](31f6112))
* mock shared-runner for improved test isolation in main.test.ts ([a1bfac2](a1bfac2))
* move magic numbers to Sidequest config ([#147](https://github.com/sidequestjs/sidequest/issues/147)) ([f701f1a](f701f1a))
* moving args to run function ([225bee3](225bee3))
* realising staled claimed and running jobs ([#59](#59)) ([148f64c](148f64c))
* recurring jobs ([#12](#12)) ([cd66663](cd66663))
* redis backend ([#129](https://github.com/sidequestjs/sidequest/issues/129)) ([f08e9b7](f08e9b7))
* refactor backend import paths and introduce setTestBackend function ([f5439da](f5439da))
* refactor uniquiness ([#67](#67)) ([fbea055](fbea055))
* removed dev server ([ffe153d](ffe153d))
* rerun jobs and fixing small issues ([6e2925c](6e2925c))
* routines ([6abf0d4](6abf0d4))
* run from dashboard ([b65b058](b65b058))
* unlimited jobs ([#21](#21)) ([0055f4a](0055f4a))
* Update branch triggers to include 'develop' for workflows ([#1](#1)) ([eef8a35](eef8a35))
* Update job arguments type to array in SidequestDashboard ([a920c27](a920c27))

### Reverts

* Revert "Chore/fix env ([#166](https://github.com/sidequestjs/sidequest/issues/166))" ([#167](https://github.com/sidequestjs/sidequest/issues/167)) ([6254959](6254959))
@sidequest-release
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@GiovaniGuizzo GiovaniGuizzo GiovaniGuizzo approved these changes

Assignees
No one assigned
Labels
released on @next released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@merencia @GiovaniGuizzo