Add support to set queue options, either globally using queue.php or by implementing an interface #159
Conversation
WalkthroughA new interface for queue options was introduced, and the RoadRunner queue system was enhanced to allow setting queue options globally or per job. The connector and queue classes were updated to handle these options, enabling dynamic configuration such as Kafka topic selection. The changelog was updated to reflect these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Connector as RoadRunnerConnector
participant Queue as RoadRunnerQueue
participant Job as Job (implements HasQueueOptions?)
participant RRQueue as RoadRunner Queue Driver
App->>Connector: connect(config)
Connector->>Queue: new RoadRunnerQueue(..., config['options'] ?? [])
App->>Queue: push(job)
Queue->>Job: queueOptions() (if HasQueueOptions)
Queue->>RRQueue: getQueue(queue, mergedOptions)
RRQueue-->>Queue: QueueInterface
Queue->>RRQueue: dispatch job with options
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.17)Note: Using configuration file /phpstan.neon.dist. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/Queue/RoadRunnerQueue.php (1)
147-161: Consider documenting the auto_ack default behaviorThe
auto_ackparameter defaults tofalse, which means messages won't be automatically acknowledged. This could lead to message redelivery if not handled properly by the job processor.Consider adding a comment explaining this behavior or documenting it in the configuration file to ensure developers are aware of the implications:
private function getQueueOptions(array $overrides = []): OptionsInterface { $config = array_merge($this->defaultOptions, $overrides); + // Note: auto_ack defaults to false, requiring manual acknowledgment of messages $options = new Options( $config['delay'] ?? 0, $config['priority'] ?? 0, $config['auto_ack'] ?? false );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)src/Queue/Contract/HasQueueOptions.php(1 hunks)src/Queue/RoadRunnerConnector.php(1 hunks)src/Queue/RoadRunnerQueue.php(6 hunks)
🔇 Additional comments (2)
CHANGELOG.md (1)
10-10: LGTM!The changelog entry is properly formatted and clearly describes the new feature.
src/Queue/RoadRunnerConnector.php (1)
25-30: LGTM!The implementation correctly passes the options configuration to the RoadRunnerQueue constructor with a sensible default (empty array) when options are not configured.
| interface HasQueueOptions | ||
| { | ||
| function queueOptions(): array; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add explicit visibility modifier and return type declaration
While interface methods are implicitly public in PHP, it's a best practice to explicitly declare the visibility modifier. Additionally, add a return type declaration for better type safety.
interface HasQueueOptions
{
- function queueOptions(): array;
+ public function queueOptions(): array;
}Also consider adding PHPDoc to document the expected structure of the returned array:
interface HasQueueOptions
{
/**
* Get queue options for this job.
*
* @return array{
* driver?: string,
* delay?: int,
* priority?: int,
* auto_ack?: bool,
* topic?: string
* }
*/
public function queueOptions(): array;
}🤖 Prompt for AI Agents
In src/Queue/Contract/HasQueueOptions.php lines 5 to 8, explicitly add the
public visibility modifier to the queueOptions method and declare its return
type as array. Additionally, include a PHPDoc block above the method to document
the expected structure of the returned array, specifying optional keys like
driver, delay, priority, auto_ack, and topic with their respective types for
better clarity and type safety.
|
Maybe it's better to use |
The reason to return array is so we can merge it with the global options |
There is What do you think? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Queue/RoadRunnerQueue.php (1)
159-162: Address the unused parameter warning.The static analysis correctly identifies that the
$queueparameter is unused. Since this method throws an exception and doesn't use the parameter, consider removing it or adding a comment explaining why it's kept for interface compliance.- public function pop($queue = null): void + public function pop($queue = null): void // @phpmd-ignore-param $queue - kept for interface complianceOr if the parameter isn't required by the interface:
- public function pop($queue = null): void + public function pop(): void
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Queue/Contract/HasQueueOptions.php(1 hunks)src/Queue/RoadRunnerQueue.php(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Queue/RoadRunnerQueue.php (2)
src/Queue/RoadRunnerConnector.php (1)
connect(19-31)src/Queue/Contract/HasQueueOptions.php (1)
queueOptions(9-9)
🪛 PHPMD (2.15.0)
src/Queue/RoadRunnerQueue.php
159-159: Avoid unused parameters such as '$queue'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (13)
src/Queue/Contract/HasQueueOptions.php (1)
7-10: LGTM! Good design choice with OptionsInterface return type.The interface is well-designed with proper type safety. Returning
OptionsInterfaceinstead of a plain array (as discussed in PR comments) is the correct approach as it provides better type safety and leverages existing merge capabilities in the Options classes.src/Queue/RoadRunnerQueue.php (12)
15-20: LGTM! Proper imports for queue options functionality.The new imports are correctly added to support the queue options feature, including the necessary classes from RoadRunner Jobs and the new HasQueueOptions interface.
28-30: LGTM! Constructor properly extended for default options.The constructor correctly accepts the new
$defaultOptionsparameter with appropriate default value and readonly visibility modifier.
39-39: LGTM! Proper integration of job-specific options in push method.The push method correctly retrieves job-specific options using
getJobOverrideOptionsand passes them topushRaw. This enables per-job queue option customization as intended.
45-45: LGTM! Options parameter properly passed to getQueue.The
pushRawmethod correctly passes the options array togetQueuefor processing.
55-64: LGTM! Enhanced queue connection with options and auto-resume.The
getQueuemethod properly:
- Merges options using
getQueueOptions- Checks queue readiness using stats
- Auto-resumes queue if not ready
This improves reliability and supports the new options functionality.
66-80: LGTM! Well-implemented queue options handling with driver-specific support.The
getQueueOptionsmethod:
- Properly merges default and override options
- Creates appropriate Options instance with sensible defaults
- Supports driver-specific options (Kafka with topic)
- Uses match expression for clean driver selection
82-96: LGTM! Proper stats retrieval with fallback.The
getStatsmethod correctly:
- Handles null queue parameter with default
- Iterates through stats to find matching pipeline
- Returns empty Stat as fallback
121-121: LGTM! Consistent job options handling in later method.The
latermethod correctly applies the same job options pattern as thepushmethod, ensuring consistency across delayed job scheduling.
132-134: LGTM! Proper options handling in laterRaw method.The
laterRawmethod correctly accepts and passes options togetQueue, maintaining the options chain for delayed jobs.
150-157: LGTM! Improved availableAt method with better type hints.The method is now properly documented and handles different delay types correctly. The protected visibility is appropriate for potential inheritance.
164-169: LGTM! Proper queue size calculation using stats.The
sizemethod correctly uses the newgetStatsmethod and calculates total queue size by combining active and delayed jobs.
98-112: Confirm presence of Options::toArray()
I searched the local codebase and didn’t find atoArray()method on anyOptionsclass. IfOptionscomes from an external dependency (e.g. Spiral\RoadRunner\Jobs\Options), please:
- Verify that the imported
Optionsclass definestoArray().- If it does not, add or polyfill that method (or adjust this call) to prevent a fatal error at runtime.
File needing attention:
• src/Queue/RoadRunnerQueue.php (lines 98–112)
Description
This PR adds support to set queue options, both globally and per job.
Or by implementing
HasQueueOptionsinterface on the job:Fixes #158
Checklist
CHANGELOG.mdfileSummary by CodeRabbit