gmr/pamqp

Unnecessary `Optional` typing

decaz opened this issue · 1 comments

decaz commented

For example Queue.DeclareOk has None as the default value of parameters:

pamqp/pamqp/commands.py

Lines 1309 to 1313 in 0cdcca9

def __init__(self,
queue: typing.Optional[str] = None,
message_count: typing.Optional[int] = None,
consumer_count: typing.Optional[int] = None) -> None:
"""Initialize the :class:`Queue.DeclareOk` class"""

But due to the reference all these values cannot be None.

gmr commented

The optional nature is based upon the AMQP spec which does go as far as to specify if fields are required or not. In the codegen, we look for the required XML attribute or JSON attribute.

I agree in theory these will never be NULL and I do see queue has an explicit <assert check = "notnull" /> child in the XML.

I'm going to defer this to 4.0 though since it will significantly change the code in commands.py and require thorough investigation for each frame time to ensure that the intention is correctly determined. For example, in Queue declare-ok, we have a non-null assertion for queue, but not for message_count or consumer_count. What should we imply the behavior to be when the spec explicitly calls out required fields and assertions for non-null fields, specifies default values, yet ignores message_count and consumer_count.