From 1054850ee1dd8d24858d0a1b965cf67b47bfe995 Mon Sep 17 00:00:00 2001 From: Hector Oliveros Date: Tue, 21 Jan 2025 13:54:46 -0300 Subject: [PATCH 1/3] Update connection limit logging to check for changes Added conditionals to log limit updates only when changes are detected, reducing unnecessary log noise. This applies to both server and client limit updates in the connection logic. --- asyncua/common/connection.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/asyncua/common/connection.py b/asyncua/common/connection.py index 84376a3ec..f10ec4144 100644 --- a/asyncua/common/connection.py +++ b/asyncua/common/connection.py @@ -54,11 +54,18 @@ def create_acknowledge_and_set_limits(self, msg: ua.Hello) -> ua.Acknowledge: ack.SendBufferSize = min(msg.SendBufferSize, self.max_recv_buffer) ack.MaxChunkCount = self._select_limit(msg.MaxChunkCount, self.max_chunk_count) ack.MaxMessageSize = self._select_limit(msg.MaxMessageSize, self.max_message_size) + have_changes = ( + self.max_chunk_count != ack.MaxChunkCount or + self.max_recv_buffer != ack.ReceiveBufferSize or + self.max_send_buffer != ack.SendBufferSize or + self.max_message_size != ack.MaxMessageSize + ) + if have_changes: + _logger.info("updating server limits to: %s", self) self.max_chunk_count = ack.MaxChunkCount self.max_recv_buffer = ack.SendBufferSize self.max_send_buffer = ack.ReceiveBufferSize self.max_message_size = ack.MaxMessageSize - _logger.info("updating server limits to: %s", self) return ack def create_hello_limits(self, msg: ua.Hello) -> ua.Hello: @@ -69,6 +76,14 @@ def create_hello_limits(self, msg: ua.Hello) -> ua.Hello: return msg def update_client_limits(self, msg: ua.Acknowledge) -> None: + have_changes = ( + self.max_chunk_count != msg.MaxChunkCount or + self.max_recv_buffer != msg.ReceiveBufferSize or + self.max_send_buffer != msg.SendBufferSize or + self.max_message_size != msg.MaxMessageSize + ) + if have_changes: + _logger.info("updating client limits to: %s", self) self.max_chunk_count = msg.MaxChunkCount self.max_recv_buffer = msg.ReceiveBufferSize self.max_send_buffer = msg.SendBufferSize From 1b840afeb74ea8970dc000c46ddec85a8cbb0407 Mon Sep 17 00:00:00 2001 From: Hector Oliveros Date: Tue, 21 Jan 2025 13:58:27 -0300 Subject: [PATCH 2/3] Fix indentation in server and client limit updates Adjusted indentation in the `update_limits` methods to ensure values are updated only when changes are detected. This aligns the logic with the condition and avoids unnecessary updates. --- asyncua/common/connection.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/asyncua/common/connection.py b/asyncua/common/connection.py index f10ec4144..ffea85186 100644 --- a/asyncua/common/connection.py +++ b/asyncua/common/connection.py @@ -62,10 +62,10 @@ def create_acknowledge_and_set_limits(self, msg: ua.Hello) -> ua.Acknowledge: ) if have_changes: _logger.info("updating server limits to: %s", self) - self.max_chunk_count = ack.MaxChunkCount - self.max_recv_buffer = ack.SendBufferSize - self.max_send_buffer = ack.ReceiveBufferSize - self.max_message_size = ack.MaxMessageSize + self.max_chunk_count = ack.MaxChunkCount + self.max_recv_buffer = ack.SendBufferSize + self.max_send_buffer = ack.ReceiveBufferSize + self.max_message_size = ack.MaxMessageSize return ack def create_hello_limits(self, msg: ua.Hello) -> ua.Hello: @@ -84,11 +84,10 @@ def update_client_limits(self, msg: ua.Acknowledge) -> None: ) if have_changes: _logger.info("updating client limits to: %s", self) - self.max_chunk_count = msg.MaxChunkCount - self.max_recv_buffer = msg.ReceiveBufferSize - self.max_send_buffer = msg.SendBufferSize - self.max_message_size = msg.MaxMessageSize - _logger.info("updating client limits to: %s", self) + self.max_chunk_count = msg.MaxChunkCount + self.max_recv_buffer = msg.ReceiveBufferSize + self.max_send_buffer = msg.SendBufferSize + self.max_message_size = msg.MaxMessageSize class MessageChunk: From df595505c38abf12b00d74098ad3db025c43c779 Mon Sep 17 00:00:00 2001 From: Hector Oliveros Date: Tue, 21 Jan 2025 16:46:49 -0300 Subject: [PATCH 3/3] Fix ruff format --- asyncua/client/client.py | 2 +- asyncua/client/ha/reconciliator.py | 2 +- asyncua/common/connection.py | 21 ++++++++++----------- asyncua/common/copy_node_util.py | 2 +- asyncua/common/utils.py | 2 +- asyncua/common/xmlimporter.py | 2 +- asyncua/server/history_sql.py | 4 ++-- asyncua/server/internal_session.py | 3 +-- asyncua/server/user_managers.py | 2 +- asyncua/ua/uatypes.py | 2 +- tests/test_crypto_connect.py | 2 +- 11 files changed, 21 insertions(+), 23 deletions(-) diff --git a/asyncua/client/client.py b/asyncua/client/client.py index b9e00ccc4..4a6392955 100644 --- a/asyncua/client/client.py +++ b/asyncua/client/client.py @@ -796,7 +796,7 @@ def get_subscription_revised_params( and new_keepalive_count != params.RequestedMaxKeepAliveCount ): _logger.info( - "KeepAliveCount will be updated to %s " "for consistency with RevisedPublishInterval", + "KeepAliveCount will be updated to %s for consistency with RevisedPublishInterval", new_keepalive_count, ) modified_params = ua.ModifySubscriptionParameters() diff --git a/asyncua/client/ha/reconciliator.py b/asyncua/client/ha/reconciliator.py index 78bb5f93b..f18073443 100644 --- a/asyncua/client/ha/reconciliator.py +++ b/asyncua/client/ha/reconciliator.py @@ -219,7 +219,7 @@ async def update_nodes(self, real_map: SubMap, ideal_map: SubMap, targets: Set[s # in case the previous create_subscription request failed if not real_sub: _logger.warning( - "Can't create nodes for %s since underlying " "subscription for %s doesn't exist", url, sub_name + "Can't create nodes for %s since underlying subscription for %s doesn't exist", url, sub_name ) continue vs_real = real_map[url][sub_name] diff --git a/asyncua/common/connection.py b/asyncua/common/connection.py index ffea85186..39d70ac34 100644 --- a/asyncua/common/connection.py +++ b/asyncua/common/connection.py @@ -55,10 +55,10 @@ def create_acknowledge_and_set_limits(self, msg: ua.Hello) -> ua.Acknowledge: ack.MaxChunkCount = self._select_limit(msg.MaxChunkCount, self.max_chunk_count) ack.MaxMessageSize = self._select_limit(msg.MaxMessageSize, self.max_message_size) have_changes = ( - self.max_chunk_count != ack.MaxChunkCount or - self.max_recv_buffer != ack.ReceiveBufferSize or - self.max_send_buffer != ack.SendBufferSize or - self.max_message_size != ack.MaxMessageSize + self.max_chunk_count != ack.MaxChunkCount + or self.max_recv_buffer != ack.ReceiveBufferSize + or self.max_send_buffer != ack.SendBufferSize + or self.max_message_size != ack.MaxMessageSize ) if have_changes: _logger.info("updating server limits to: %s", self) @@ -77,10 +77,10 @@ def create_hello_limits(self, msg: ua.Hello) -> ua.Hello: def update_client_limits(self, msg: ua.Acknowledge) -> None: have_changes = ( - self.max_chunk_count != msg.MaxChunkCount or - self.max_recv_buffer != msg.ReceiveBufferSize or - self.max_send_buffer != msg.SendBufferSize or - self.max_message_size != msg.MaxMessageSize + self.max_chunk_count != msg.MaxChunkCount + or self.max_recv_buffer != msg.ReceiveBufferSize + or self.max_send_buffer != msg.SendBufferSize + or self.max_message_size != msg.MaxMessageSize ) if have_changes: _logger.info("updating client limits to: %s", self) @@ -384,8 +384,7 @@ def _check_sym_header(self, security_hdr): ) if timeout < datetime.now(timezone.utc): raise ua.UaError( - f"Security token id {security_hdr.TokenId} has timed out " - f"({timeout} < {datetime.now(timezone.utc)})" + f"Security token id {security_hdr.TokenId} has timed out ({timeout} < {datetime.now(timezone.utc)})" ) return @@ -400,7 +399,7 @@ def _check_incoming_chunk(self, chunk): if chunk.MessageHeader.MessageType != ua.MessageType.SecureOpen: if chunk.MessageHeader.ChannelId != self.security_token.ChannelId: raise ua.UaError( - f"Wrong channel id {chunk.MessageHeader.ChannelId}," f" expected {self.security_token.ChannelId}" + f"Wrong channel id {chunk.MessageHeader.ChannelId}, expected {self.security_token.ChannelId}" ) if self._incoming_parts: if self._incoming_parts[0].SequenceHeader.RequestId != chunk.SequenceHeader.RequestId: diff --git a/asyncua/common/copy_node_util.py b/asyncua/common/copy_node_util.py index 7335677a7..f6eff8a74 100644 --- a/asyncua/common/copy_node_util.py +++ b/asyncua/common/copy_node_util.py @@ -118,7 +118,7 @@ async def _read_and_copy_attrs(node_type: asyncua.Node, struct: Any, addnode: ua setattr(struct, name, variant.Value) else: _logger.warning( - "Instantiate: while copying attributes from node type %s," " attribute %s, statuscode is %s", + "Instantiate: while copying attributes from node type %s, attribute %s, statuscode is %s", str(node_type), str(name), str(results[idx].StatusCode), diff --git a/asyncua/common/utils.py b/asyncua/common/utils.py index 0f9547d05..69202ad5e 100644 --- a/asyncua/common/utils.py +++ b/asyncua/common/utils.py @@ -43,7 +43,7 @@ def __init__(self, data, start_pos=0, size=-1): self._size = size def __str__(self): - return f"Buffer(size:{self._size}, data:{self._data[self._cur_pos:self._cur_pos + self._size]})" + return f"Buffer(size:{self._size}, data:{self._data[self._cur_pos : self._cur_pos + self._size]})" __repr__ = __str__ diff --git a/asyncua/common/xmlimporter.py b/asyncua/common/xmlimporter.py index 901b0eedf..9e99c938e 100644 --- a/asyncua/common/xmlimporter.py +++ b/asyncua/common/xmlimporter.py @@ -669,7 +669,7 @@ async def add_datatype(self, obj, no_namespace_migration=False): is_struct = True else: _logger.warning( - "%s has datatypedefinition and path %s" " but we could not find out if this is a struct", + "%s has datatypedefinition and path %s but we could not find out if this is a struct", obj, path, ) diff --git a/asyncua/server/history_sql.py b/asyncua/server/history_sql.py index f27e877df..28def9a88 100644 --- a/asyncua/server/history_sql.py +++ b/asyncua/server/history_sql.py @@ -112,7 +112,7 @@ async def read_node_history(self, node_id, start, end, nb_values): try: validate_table_name(table) async with self._db.execute( - f'SELECT * FROM "{table}" WHERE "SourceTimestamp" BETWEEN ? AND ? ' f'ORDER BY "_Id" {order} LIMIT ?', + f'SELECT * FROM "{table}" WHERE "SourceTimestamp" BETWEEN ? AND ? ORDER BY "_Id" {order} LIMIT ?', ( start_time, end_time, @@ -294,7 +294,7 @@ def _get_select_clauses(self, source_id, evfilter): s_clauses.append(name) except AttributeError: self.logger.warning( - "Historizing SQL OPC UA Select Clause Warning for node %s," " Clause: %s:", source_id, select_clause + "Historizing SQL OPC UA Select Clause Warning for node %s, Clause: %s:", source_id, select_clause ) # remove select clauses that the event type doesn't have; SQL will error because the column doesn't exist clauses = [x for x in s_clauses if x in self._event_fields[source_id]] diff --git a/asyncua/server/internal_session.py b/asyncua/server/internal_session.py index 6c171f5f1..9b46666ee 100644 --- a/asyncua/server/internal_session.py +++ b/asyncua/server/internal_session.py @@ -59,8 +59,7 @@ def __init__( def __str__(self): return ( - f"InternalSession(name:{self.name}," - f" user:{self.user}, id:{self.session_id}, auth_token:{self.auth_token})" + f"InternalSession(name:{self.name}, user:{self.user}, id:{self.session_id}, auth_token:{self.auth_token})" ) async def get_endpoints(self, params=None, sockname=None): diff --git a/asyncua/server/user_managers.py b/asyncua/server/user_managers.py index 9e41f8963..0f54d1bd6 100644 --- a/asyncua/server/user_managers.py +++ b/asyncua/server/user_managers.py @@ -39,7 +39,7 @@ async def add_role(self, certificate_path: Path, user_role: UserRole, name: str, if name in self._trusted_certificates: logging.warning( - "certificate with name %s " "attempted to be added multiple times, only the last version will be kept.", + "certificate with name %s attempted to be added multiple times, only the last version will be kept.", name, ) self._trusted_certificates[name] = {"certificate": uacrypto.der_from_x509(certificate), "user": user} diff --git a/asyncua/ua/uatypes.py b/asyncua/ua/uatypes.py index 3418c74c9..57e41c752 100644 --- a/asyncua/ua/uatypes.py +++ b/asyncua/ua/uatypes.py @@ -786,7 +786,7 @@ def __init__(self, Text=None, Locale=None): if self.Text is not None: if not isinstance(self.Text, str): raise ValueError( - f'A LocalizedText object takes a string as argument "text"' f"not a {type(self.Text)}, {self.Text}" + f'A LocalizedText object takes a string as argument "text"not a {type(self.Text)}, {self.Text}' ) if self.Locale is not None: diff --git a/tests/test_crypto_connect.py b/tests/test_crypto_connect.py index 2d549c29e..f64f9718d 100644 --- a/tests/test_crypto_connect.py +++ b/tests/test_crypto_connect.py @@ -503,7 +503,7 @@ async def test_security_level_endpoints(srv_crypto_all_certs: Tuple[Server, str] policy_type = ua.SecurityPolicyType.NoSecurity else: policy_type = ua.SecurityPolicyType[ - f'{end_point.SecurityPolicyUri.split("#")[1].replace("_", "")}_{end_point.SecurityMode.name}' + f"{end_point.SecurityPolicyUri.split('#')[1].replace('_', '')}_{end_point.SecurityMode.name}" ] assert end_point.SecurityLevel == SECURITY_POLICY_TYPE_MAP[policy_type][2]