-
Notifications
You must be signed in to change notification settings - Fork 506
Add int ranges to AMQP typings #3146
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
Conversation
resources/functionMap.php
Outdated
@@ -72,10 +72,10 @@ | |||
'AMQPChannel::__construct' => ['void', 'connection'=>'AMQPConnection'], | |||
'AMQPChannel::basicRecover' => ['void', 'requeue='=>'bool'], | |||
'AMQPChannel::commitTransaction' => ['void'], | |||
'AMQPChannel::getChannelId' => ['int'], | |||
'AMQPChannel::getChannelId' => ['int<1,65535>'], |
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.
This is defined as a 16bit unsigned int in the spec, but setting to 0 triggers an error always, so cannot be returned from the getter.
resources/functionMap.php
Outdated
'AMQPChannel::getConnection' => ['AMQPConnection'], | ||
'AMQPChannel::getPrefetchCount' => ['int'], | ||
'AMQPChannel::getPrefetchSize' => ['int'], | ||
'AMQPChannel::getPrefetchCount' => ['int<0,65535>'], |
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.
Similarly a 16bit unsigned int, zero signifies no prefetch.
resources/functionMap.php
Outdated
'AMQPChannel::getPrefetchCount' => ['int'], | ||
'AMQPChannel::getPrefetchSize' => ['int'], | ||
'AMQPChannel::getPrefetchCount' => ['int<0,65535>'], | ||
'AMQPChannel::getPrefetchSize' => ['int<0,max>'], |
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.
Octets, zero signifying maximum possible.
resources/functionMap.php
Outdated
@@ -87,12 +87,12 @@ | |||
'AMQPConnection::disconnect' => ['void'], | |||
'AMQPConnection::getHost' => ['string'], | |||
'AMQPConnection::getLogin' => ['string'], | |||
'AMQPConnection::getMaxChannels' => ['int'], | |||
'AMQPConnection::getMaxChannels' => ['int<1,65535>'], |
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.
Similar to above, a channel of 0 is invalid, despite being a 16bit unsigned int.
resources/functionMap.php
Outdated
@@ -121,7 +121,7 @@ | |||
'AMQPEnvelope::getHeader' => ['mixed', 'headerName'=>'string'], | |||
'AMQPEnvelope::getHeaders' => ['array<string,mixed>'], | |||
'AMQPEnvelope::getMessageId' => ['string|null'], | |||
'AMQPEnvelope::getPriority' => ['int'], | |||
'AMQPEnvelope::getPriority' => ['int<0,max>'], |
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.
AMQP spec defines that a priority MAY be 0-10, where 0 is "default". Above 10 is undefined. RabbitMQ supports up to 255, this value is capped to the maximum supported by the queue, hence leaving it leniant.
@ondrejmirtes Can we have some coding style directives on the function map signatures? I always use Maybe we add a couple of lines on top of the file saying this? I also would like to see a specification on whether to use |
Thank you. |
I don't care too much about formatting inside these types. Whatever works, use that :) |
I haven't changed the setters as this is more of an annoyance to call site. It will trigger a runtime error if out of bounds. Happy to update setters as well if preferred!