sts-sponsors team mailing list archive

[Björn Tillenius <mp+438520@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing



Diff comments:

> diff --git a/src/maasserver/websockets/protocol.py b/src/maasserver/websockets/protocol.py
> index 57c5f33..eed8345 100644
> --- a/src/maasserver/websockets/protocol.py
> +++ b/src/maasserver/websockets/protocol.py
> @@ -265,27 +247,30 @@ class WebSocketProtocol(Protocol):
>                  return handledMessages
>          return handledMessages
>  
> +    @inlineCallbacks
>      def handleRequest(self, message, msg_type=MSG_TYPE.REQUEST):

Looks like we're missing some test coverage here (unless I'm missing something?)

You change this from either return None or a deferred to always returning a deferred. However, the callsite checks for 'handleRequest(...) is None', which now never will be True.

>          """Handle the request message."""
>          # Get the required request_id.
>          request_id = self.getMessageField(message, "request_id")
>          if request_id is None:
> -            return None
> +            returnValue(None)
> +            return
>  
>          if msg_type == MSG_TYPE.PING:
>              self.sequence_number += 1
> -            return defer.succeed(
> -                self.sendResult(
> -                    request_id=request_id,
> -                    result=self.sequence_number,
> -                    msg_type=MSG_TYPE.PING_REPLY,
> -                )
> +            self.sendResult(
> +                request_id=request_id,
> +                result=self.sequence_number,
> +                msg_type=MSG_TYPE.PING_REPLY,
>              )
> +            returnValue(None)
> +            return
>  
>          # Decode the method to be called.
>          msg_method = self.getMessageField(message, "method")
>          if msg_method is None:
> -            return None
> +            returnValue(None)
> +            return
>          try:
>              handler_name, method = msg_method.split(".", 1)
>          except ValueError:


-- 
https://code.launchpad.net/~ack/maas/+git/maas/+merge/438520
Your team MAAS Committers is subscribed to branch maas:master.