-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add additional options when creating a pipeline #60
Conversation
WalkthroughThe changes across the codebase primarily focus on enhancing the functionality of various queue creation classes. New properties and constants have been added to these classes, with corresponding updates to their constructors, assertion checks, and Changes
Poem
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (8)
- src/Queue/AMQPCreateInfo.php (4 hunks)
- src/Queue/BeanstalkCreateInfo.php (3 hunks)
- src/Queue/BoltdbCreateInfo.php (3 hunks)
- src/Queue/SQSCreateInfo.php (3 hunks)
- tests/Unit/Queue/AMQPCreateInfoTest.php (4 hunks)
- tests/Unit/Queue/BeanstalkCreateInfoTest.php (4 hunks)
- tests/Unit/Queue/BoltdbCreateInfoTest.php (3 hunks)
- tests/Unit/Queue/SQSCreateInfoTest.php (8 hunks)
Additional comments: 31
tests/Unit/Queue/AMQPCreateInfoTest.php (3)
- 25-64: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [14-36]
The test
testDefaultValues
checks the default values of the new properties added to theAMQPCreateInfo
class. This is a good practice to ensure that the default values are as expected.
- 25-64: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [38-81]
The test
testCustomValues
checks the custom values of the new properties added to theAMQPCreateInfo
class. This is a good practice to ensure that the custom values are set correctly.
- 73-109: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [83-134]
The test
testToArray
checks thetoArray
method of theAMQPCreateInfo
class. This is a good practice to ensure that thetoArray
method returns the expected array representation of theAMQPCreateInfo
object.tests/Unit/Queue/BoltdbCreateInfoTest.php (3)
15-26: The test for the constructor is correctly updated to include the new
$permissions
property. It checks that the value passed to the constructor is correctly set on the object.33-36: The test for default values is correctly updated to include the new
$permissions
property. It checks that the default value is correctly set when no value is passed to the constructor.51-57: The test for the
toArray
method is correctly updated to include the new$permissions
property. It checks that the array representation of the object includes the correct value for$permissions
.src/Queue/BeanstalkCreateInfo.php (3)
13-17: The new constant
CONSUME_ALL_DEFAULT_VALUE
is defined correctly and initialized with a default value offalse
.28-32: The new property
$consumeAll
is added to the constructor and is initialized with the default valueCONSUME_ALL_DEFAULT_VALUE
. Ensure that all instances ofBeanstalkCreateInfo
are updated to include this new property if necessary.43-47: The
toArray()
method is updated to include theconsume_all
key in the returned array. This is consistent with the rest of the code.src/Queue/SQSCreateInfo.php (4)
33-37: New constants for default values of
messageGroupId
andskipQueueDeclaration
are added. Ensure that these default values are appropriate for all use cases.45-51: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [45-60]
The constructor now accepts two new parameters
messageGroupId
andskipQueueDeclaration
. Make sure all instances of this class are updated to reflect these changes.
64-70: Precondition checks are added for the new parameters. This is a good practice to ensure the validity of the input data.
73-93: The
toArray()
method is updated to include the new properties in the resulting array. This is consistent with the changes made to the class.src/Queue/AMQPCreateInfo.php (5)
10-16: New constants have been added to the class. Ensure that these constants are used correctly throughout the codebase.
20-26: More constants have been added. As before, verify their usage.
33-37: New parameters have been added to the constructor. Ensure that all instances of this class are created with the correct arguments.
51-98: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [39-68]
The constructor has been updated to include the new properties. The assertions are a good way to ensure that the properties are valid at the time of object creation.
- 71-97: The
toArray()
method has been updated to include the new properties. This is a good practice as it ensures that the array representation of the object is always up-to-date.tests/Unit/Queue/BeanstalkCreateInfoTest.php (3)
21-24: The test checks the default value of the new property
consumeAll
. This is a good practice to ensure that the default values are as expected.45-51: The test checks the custom value of the new property
consumeAll
. This is a good practice to ensure that the custom values are set correctly.82-82: The test checks that the new property
consumeAll
is included in the array returned by thetoArray
method. This is a good practice to ensure that thetoArray
method works as expected.tests/Unit/Queue/SQSCreateInfoTest.php (8)
- 13-28: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [13-39]
The testConstructor method has been updated to include the new properties
messageGroupId
andskipQueueDeclaration
. The assertions for these properties are correct.
- 34-54: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [41-68]
The testToArray method has been updated to include the new property
skipQueueDeclaration
. The assertion for this property is correct.
82-96: The testCreateWithTags method has been updated to include the new property
skipQueueDeclaration
. The assertion for this property is correct.107-121: The testCreateWithAttributes method has been updated to include the new property
skipQueueDeclaration
. The assertion for this property is correct.131-139: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [123-139]
The testToArrayWithDefaults method has been updated to include the new property
skipQueueDeclaration
. The assertion for this property is correct.
152-167: The testCreateWithTagsAndAttributes method has been updated to include the new property
skipQueueDeclaration
. The assertion for this property is correct.176-182: The testCreateWithoutTagsAndAttributes method has been updated to include the new property
skipQueueDeclaration
. The assertion for this property is correct.185-200: A new test method, testCreateWithMessageGroupId, has been added to test the
messageGroupId
property. The assertion for this property is correct.src/Queue/BoltdbCreateInfo.php (2)
24-27: The new
$permissions
property is added to the constructor with a default value ofPERMISSIONS_DEFAULT_VALUE
. Ensure that all calls to this constructor throughout the codebase have been updated to match the new signature.37-41: The
toArray()
method is updated to include the new$permissions
property. This looks good.
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## 4.x #60 +/- ##
============================================
+ Coverage 86.56% 86.87% +0.31%
- Complexity 191 195 +4
============================================
Files 30 30
Lines 588 602 +14
============================================
+ Hits 509 523 +14
Misses 79 79
☔ View full report in Codecov by Sentry. |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/Queue/BoltdbCreateInfo.php (3 hunks)
- tests/Unit/Queue/BoltdbCreateInfoTest.php (3 hunks)
Additional comments: 6
src/Queue/BoltdbCreateInfo.php (3)
11-14: The new constant
PERMISSIONS_DEFAULT_VALUE
is correctly defined and initialized.24-27: The new property
$permissions
is correctly initialized with the default value.37-41: The
toArray()
method correctly includes the new'permissions'
key in the returned array.tests/Unit/Queue/BoltdbCreateInfoTest.php (3)
15-26: The test for the constructor is well written and checks all the properties of the
BoltdbCreateInfo
object. It's good to see that the new$permissions
property is also being tested.33-39: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [29-37]
The test for default values is also well written and checks all the default values of the
BoltdbCreateInfo
object. The new$permissions
property is also being tested for its default value.
- 51-59: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [39-58]
The
testToArray
method is correctly updated to include the newpermissions
property in the expected array. The test checks if thetoArray
method returns the expected array representation of theBoltdbCreateInfo
object.
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
This pull request adds additional options when creating a pipeline.
AMQP
These options are available in the driver configuration: https://github.com/roadrunner-server/amqp/blob/master/amqpjobs/config.go#L24 and in the configuration file
.rr.yaml
:https://github.com/roadrunner-server/roadrunner/blob/master/.rr.yaml#L1402
Beanstalk
This option is available in the driver configuration: https://github.com/roadrunner-server/beanstalk/blob/master/beanstalkjobs/config.go#L11 and in the configuration file
.rr.yaml
:https://github.com/roadrunner-server/roadrunner/blob/master/.rr.yaml#L1521
Boltdb
This option is available in the driver configuration: https://github.com/roadrunner-server/boltdb/blob/master/boltjobs/config.go#L6 and in the configuration file
.rr.yaml
:https://github.com/roadrunner-server/roadrunner/blob/master/.rr.yaml#L1391
SQS
Changed parameter from
wait_time_seconds
towait_time
. See: https://github.com/roadrunner-server/sqs/blob/master/sqsjobs/config.go#L16These options are available in the driver configuration: https://github.com/roadrunner-server/sqs/blob/master/sqsjobs/config.go#L9 and in the configuration file
.rr.yaml
: https://github.com/roadrunner-server/roadrunner/blob/master/.rr.yaml#L1542Summary by CodeRabbit