Skip to content
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

various cleanups for typing stuff #19270

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 9 additions & 22 deletions src/cockpit/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,48 +103,35 @@ def control_received(self, data: bytes):
except (json.JSONDecodeError, JsonError) as exc:
raise CockpitProtocolError(f'control message: {exc!s}') from exc

def consume_one_frame(self, view):
def consume_one_frame(self, data: bytes) -> int:
"""Consumes a single frame from view.

Returns positive if a number of bytes were consumed, or negative if no
work can be done because of a given number of bytes missing.
"""

# Nothing to look at? Save ourselves the trouble...
if not view:
return 0

view = bytes(view)
# We know the length + newline is never more than 10 bytes, so just
# slice that out and deal with it directly. We don't have .index() on
# a memoryview, for example.
# From a performance standpoint, hitting the exception case is going to
# be very rare: we're going to receive more than the first few bytes of
# the packet in the regular case. The more likely situation is where
# we get "unlucky" and end up splitting the header between two read()s.
Comment on lines -118 to -124
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That comment is still current, other than the "we don't have .index" bit. But it's useful to explain the magic number 10 below, as well as splitting reads in between a number.

header = bytes(view[:10])
try:
newline = header.index(b'\n')
newline = data.index(b'\n')
except ValueError as exc:
if len(header) < 10:
if len(data) < 10:
# Let's try reading more
return len(header) - 10
return len(data) - 10
raise CockpitProtocolError("size line is too long") from exc

try:
length = int(header[:newline])
length = int(data[:newline])
except ValueError as exc:
raise CockpitProtocolError("frame size is not an integer") from exc

start = newline + 1
end = start + length

if end > len(view):
if end > len(data):
# We need to read more
return len(view) - end
return len(data) - end

# We can consume a full frame
self.frame_received(view[start:end])
self.frame_received(data[start:end])
return end

def connection_made(self, transport):
Expand Down Expand Up @@ -192,7 +179,7 @@ def write_control(self, _msg: 'JsonObject | None' = None, **kwargs: JsonValue) -
def data_received(self, data):
try:
self.buffer += data
while True:
while self.buffer:
result = self.consume_one_frame(self.buffer)
if result <= 0:
return
Expand Down