feature: JOBS v4 API #40
Conversation
Signed-off-by: Valery Piashchynski <[email protected]>
|
Warning Rate limit exceeded@rustatian has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 20 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent updates involve version changes in configuration files, additions and modifications to Go and PHP files to improve task handling and error management, and adjustments to linter settings. These changes aim to enhance the robustness and maintainability of the project, ensuring compatibility with newer dependencies and refining the task processing logic. Changes
Sequence Diagram(s)(The changes are too varied or simple to warrant sequence diagrams.) Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 1
Outside diff range and nitpick comments (11)
pubsubjobs/listener.go (5)
5-5: Import Error Handling PackageThe
errorspackage is imported but not used. Consider removing it if not needed.
9-9: Import Events PackageThe
eventspackage is imported but not used. Consider removing it if not needed.
15-17: Define Constants ClearlyThe
restartStrconstant is defined but not used within the provided code. Ensure it is necessary or remove it.
33-34: Initialize Headers ProperlyConsider initializing
item.headersonly if it isnilto avoid unnecessary allocations.if item.headers == nil { item.headers = make(map[string][]string, 2) }
Line range hint
51-59: Reduce Listeners CorrectlyEnsure that the listener count is reduced correctly using atomic operations.
if st != nil && st.Message() == "grpc: the client connection is closing" { if atomic.LoadUint32(&d.listeners) > 0 { atomic.AddUint32(&d.listeners, ^uint32(0)) } d.log.Debug("listener was stopped") return }tests/jobs_test.go (5)
Line range hint
3-3: Update ImportsEnsure that all imports are necessary and correctly used in the tests.
Line range hint
3-3: Update ImportsEnsure that all imports are necessary and correctly used in the tests.
Line range hint
3-3: Update ImportsEnsure that all imports are necessary and correctly used in the tests.
Line range hint
3-3: Update ImportsEnsure that all imports are necessary and correctly used in the tests.
Line range hint
3-3: Update ImportsEnsure that all imports are necessary and correctly used in the tests.
pubsubjobs/driver.go (1)
279-280: State Method Not ImplementedThe
Statemethod is not implemented. Ensure that users are aware of this limitation.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sum
Files selected for processing (16)
- .github/workflows/linters.yml (1 hunks)
- .gitignore (1 hunks)
- .golangci.yml (1 hunks)
- go.mod (2 hunks)
- plugin.go (2 hunks)
- pubsubjobs/config.go (3 hunks)
- pubsubjobs/driver.go (13 hunks)
- pubsubjobs/item.go (10 hunks)
- pubsubjobs/listener.go (3 hunks)
- tests/configs/.rr-init.yaml (3 hunks)
- tests/configs/.rr-pq.yaml (2 hunks)
- tests/go.mod (4 hunks)
- tests/jobs_test.go (2 hunks)
- tests/php_test_files/jobs/jobs_err.php (1 hunks)
- tests/php_test_files/jobs/jobs_ok.php (1 hunks)
- tests/php_test_files/jobs/jobs_ok_pq.php (1 hunks)
Files skipped from review due to trivial changes (6)
- .github/workflows/linters.yml
- .gitignore
- .golangci.yml
- go.mod
- tests/configs/.rr-init.yaml
- tests/configs/.rr-pq.yaml
Additional comments not posted (40)
tests/php_test_files/jobs/jobs_ok.php (2)
15-15: Ensure proper error handling for task acknowledgment.The
ack()method can throw exceptions. Ensure that any potential side effects or necessary cleanups are handled within the catch block.
17-17: Verify the existence and initialization of$rr.Ensure that the
$rrobject is properly initialized before being used to log errors. If not, this will cause a runtime error.tests/php_test_files/jobs/jobs_ok_pq.php (2)
16-16: Ensure proper error handling for task acknowledgment.The
ack()method can throw exceptions. Ensure that any potential side effects or necessary cleanups are handled within the catch block.
18-18: Verify the existence and initialization of$rr.Ensure that the
$rrobject is properly initialized before being used to log errors. If not, this will cause a runtime error.tests/php_test_files/jobs/jobs_err.php (6)
16-16: Ensure the header line "attempts" is present.Before calling
getHeaderLine("attempts"), verify that this header line exists to avoid potential issues.
19-19: Ensure proper error handling for task acknowledgment.The
ack()method can throw exceptions. Ensure that any potential side effects or necessary cleanups are handled within the catch block.
21-21: Verify the usage ofwithHeader.Ensure that the
withHeadermethod properly updates the task headers and that it is used correctly in this context.
22-22: Verify the usage ofwithDelay.Ensure that the
withDelaymethod properly sets the delay for the task and that it is used correctly in this context.
23-23: Verify the usage ofrequeue.Ensure that the
requeuemethod properly requeues the task and that it is used correctly in this context.
27-27: Verify the existence and initialization of$rr.Ensure that the
$rrobject is properly initialized before being used to log errors. If not, this will cause a runtime error.pubsubjobs/config.go (3)
9-14: New constants added for configuration keys.The new constants
skipTopicKey,projectIDKey,deadLetterTopic,topicKey,maxDeliveryAttempts, andpriorityKeyare added. Ensure these keys are used consistently throughout the codebase.
26-29: New fields added to theconfigstruct.The new fields
DeadLetterTopic,MaxDeliveryAttempts, andPriorityare added to theconfigstruct. Ensure these fields are correctly populated and used in the codebase.
49-52: Default values for new configuration fields.The
InitDefaultsfunction now sets default values forMaxDeliveryAttemptsifDeadLetterTopicis specified. Ensure these defaults are appropriate and documented.plugin.go (2)
61-61: DriverFromConfig method updated.The
DriverFromConfigmethod now uses thepubsubjobs.FromConfigfunction. Ensure that theFromConfigfunction handles all necessary configurations and dependencies.
65-65: DriverFromPipeline method updated.The
DriverFromPipelinemethod now uses thepubsubjobs.FromPipelinefunction. Ensure that theFromPipelinefunction handles all necessary configurations and dependencies.pubsubjobs/listener.go (6)
27-27: Context PropagationEnsure that
ctxspanis properly closed in all error paths to avoid potential memory leaks.
45-47: Handle Context Canceled ErrorThe context canceled error is being handled correctly by setting
d.listenersto 0.
66-69: Restart Pipeline on FailureThe logic to restart the pipeline on failure is correctly implemented.
73-125: Dead-Letter HandlingEnsure the dead-letter handling logic is thoroughly tested. This is a critical section that handles failed messages.
128-128: LGTM!The
atomicCtxfunction correctly initializes the context and cancel function.
Line range hint
130-130: LGTM!The
checkCtxAndCancelfunction correctly handles context cancellation based on listener count.tests/go.mod (4)
3-3: Update Go VersionThe Go version is updated from
1.22.4to1.22.5. Ensure compatibility with all dependencies.
5-5: Replace Module PathThe module path for
google-pub-subis replaced with a local path. Ensure this is intentional for local development.
8-24: Update DependenciesThe dependencies are updated to their latest versions. Ensure compatibility and test thoroughly.
Line range hint
29-97: Indirect DependenciesMultiple indirect dependencies are updated. Ensure these updates do not introduce any breaking changes.
pubsubjobs/item.go (8)
95-95: Handle JSON Marshal ErrorEnsure that the error handling for JSON marshaling is thorough.
112-112: Auto-Ack HandlingEnsure that the auto-ack logic is correctly implemented to avoid message loss.
136-171: Nack HandlingEnsure that the negative acknowledgment logic is correctly implemented to handle message requeueing and error conditions.
173-208: Requeue HandlingEnsure that the requeue logic is correctly implemented to handle message requeueing and error conditions.
276-288: Unpack MessageEnsure that the message unpacking logic is correctly implemented to handle all attributes and potential errors.
309-325: Handle Acknowledgment ResultEnsure that the acknowledgment result handling logic is correctly implemented to manage all possible statuses.
Line range hint
218-226: Convert Job to ItemEnsure that the conversion logic from
jobs.MessagetoItemis correctly implemented to handle all fields.
Line range hint
296-306: Boolean Conversion FunctionsEnsure that the boolean conversion functions are correctly implemented and used where appropriate.
tests/jobs_test.go (2)
211-211: Use Mock LoggerThe mock logger is used correctly for testing purposes.
283-290: Log AssertionsEnsure that the log assertions are correctly verifying the expected log messages.
pubsubjobs/driver.go (5)
127-129: Initialize Event BusEnsure that the event bus is correctly initialized and used.
207-209: Initialize Event BusEnsure that the event bus is correctly initialized and used.
249-249: Job Push HandlingEnsure that the job push logic is correctly implemented to handle all possible errors and conditions.
249-249: Pipeline Run HandlingEnsure that the pipeline run logic is correctly implemented to handle all possible errors and conditions.
249-249: Pipeline Pause HandlingEnsure that the pipeline pause logic is correctly implemented to handle all possible errors and conditions.
| require dirname(__DIR__) . "/vendor/autoload.php"; | ||
|
|
||
| $consumer = new Spiral\RoadRunner\Jobs\Consumer(); | ||
|
|
||
| while ($task = $consumer->waitTask()) { | ||
| try { | ||
| sleep(15); | ||
| $task->complete(); | ||
| sleep(15); |
There was a problem hiding this comment.
Consider reducing the sleep duration.
The sleep(15) call might introduce unnecessary delays. Evaluate if a shorter duration would suffice or if there are more efficient ways to handle the delay.
Signed-off-by: Valery Piashchynski <[email protected]>
Reason for This PR
Description of Changes
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.][Reviewer TODO: Verify that these criteria are met. Request changes if not]git commit -s).CHANGELOG.md.Summary by CodeRabbit
New Features
Bug Fixes
Chores
.gitignoreentries for better project management.Tests