-
Notifications
You must be signed in to change notification settings - Fork 3
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
feature: support new JOBS api #128
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 35 minutes and 43 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. WalkthroughThis update mainly focuses on version upgrades and configuration changes. Key revisions include upgrading the Changes
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 (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
go.sum
is excluded by!**/*.sum
go.work.sum
is excluded by!**/*.sum
tests/go.sum
is excluded by!**/*.sum
Files selected for processing (7)
- .github/workflows/linters.yml (1 hunks)
- .golangci.yml (3 hunks)
- boltjobs/driver.go (1 hunks)
- boltjobs/item.go (5 hunks)
- go.mod (1 hunks)
- plugin.go (1 hunks)
- tests/go.mod (2 hunks)
Files skipped from review due to trivial changes (3)
- .github/workflows/linters.yml
- boltjobs/driver.go
- go.mod
Additional comments not posted (6)
plugin.go (3)
5-5
: Import path updated to new API version.This change is necessary to utilize the updated
jobs
API in theboltdb
plugin.
Line range hint
60-62
:DriverFromConfig
method implementation looks good.The method correctly utilizes the new API for constructing a driver from configuration.
Line range hint
64-66
:DriverFromPipeline
method implementation is correct.This method properly handles the construction of a driver from a pipeline using the updated API.
.golangci.yml (1)
65-67
: Configuration update to exclude directories from linting.Adding
.github
and.git
toexclude-dirs
helps avoid unnecessary linting in these directories, which are not part of the main application code.tests/go.mod (1)
8-21
: Dependency version updates intests/go.mod
.The updates to dependencies like
github.com/roadrunner-server/api/v4
and others are essential for ensuring compatibility with the new API changes.boltjobs/item.go (1)
6-6
: Significant updates and new method implementation inboltjobs/item.go
.The introduction of
NackWithOptions
and changes to theOptions
type (e.g.,Delay
field type change) are well-implemented and align with the new API requirements.Also applies to: 12-12, 17-17, 40-40, 131-142, 198-203
Signed-off-by: Valery Piashchynski <[email protected]>
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- boltjobs/item.go (5 hunks)
Additional context used
golangci-lint
boltjobs/item.go
200-200: S1009: should omit nil check; len() for map[string][]string is defined as zero (gosimple)
GitHub Check: Golang-CI (lint)
boltjobs/item.go
[failure] 200-200:
S1009: should omit nil check; len() for map[string][]string is defined as zero (gosimple)
Additional comments not posted (3)
boltjobs/item.go (3)
131-142
: New methodNackWithOptions
implementation review.The method
NackWithOptions
adds significant flexibility to job handling by allowing conditional requeuing with a specified delay. The implementation correctly checks forAutoAck
before proceeding and handles therequeue
condition appropriately by callingRequeue
orNack
based on therequeue
flag.
150-150
: Review of modifiedNack
method.The updated
Nack
method includes comprehensive transaction logic, ensuring robust error handling and rollback scenarios. This is crucial for maintaining data integrity in failure cases.
282-282
: Verify type conversion infromJob
method.The conversion of
job.Delay()
toint
aligns with the updatedOptions
struct. It's important to ensure that this change is consistent across all usages of theDelay
field to avoid type mismatch errors.
Signed-off-by: Valery Piashchynski <[email protected]>
Signed-off-by: Valery Piashchynski <[email protected]>
Reason for This PR
ref: roadrunner-server/roadrunner#1941
Description of Changes
NackWithOptions
method.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
Chores
Refactor
New Features
NackWithOptions
method for enhanced job control.Bug Fixes