Only log when TransportLimits actually changed #1183
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
need rebase too |
rnestler
force-pushed
the
only-log-when-limits-changed
branch
from
57aff7e
to
e78417e
Compare
|
@oroulet Rebased. |
rnestler
commented
Dec 26, 2023
rnestler
force-pushed
the
only-log-when-limits-changed
branch
from
7b480a6
to
00bf33a
Compare
rnestler
commented
Dec 26, 2023
Comment on lines
+56
to
+64
| max_recv_buffer = ack.ReceiveBufferSize if role == "client" else ack.SendBufferSize | ||
| max_send_buffer = ack.SendBufferSize if role == "client" else ack.ReceiveBufferSize | ||
|
|
||
| new_limits = TransportLimits( | ||
| max_chunk_count=ack.MaxChunkCount, | ||
| max_recv_buffer=max_recv_buffer, | ||
| max_send_buffer=max_send_buffer, | ||
| max_message_size=ack.MaxMessageSize, | ||
| ) |
There was a problem hiding this comment.
I'm now not sure if it makes sense to create the new_limits here or if we should keep this in the other methods, since it differs between client and server.
There was a problem hiding this comment.
Sorry That MR disapeard into the backlog.. but yes inline would be better I think
oroulet
reviewed
Jun 21, 2024
| @@ -51,17 +52,30 @@ def is_chunk_count_within_limit(self, sz: int) -> bool: | |||
| _logger.error("Number of message chunks: %s is > configured max chunk count: %s", sz, self.max_chunk_count) | |||
| return within_limit | |||
|
|
|||
| def _set_new_limits_from_ack(self, ack: ua.Acknowledge, role: Literal["client", "server"]) -> None: | |||
There was a problem hiding this comment.
if you really need that Role, it should be an enum.
But in that case I would jsut create two methods, one for server and one for client. That is simpler
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #1181