readability - node.server is in fact an InternalSession instance

[brubbel]

code readability.

It is highly confusing to understand the code flow for the Node.server attribute, as the server is actually an "InternalSession (server)" instance. Replaced all occurrences of node.server by node.isession.

There is a fallback property which redirects the server attribute to node.isession, with a one-shot deprecation warning.

[oroulet]

The problem is that Node is also used on client side, and calling it isession there is also very confusing. Not sure what the correct wording should be. maybe connection... or simply session. but that is a very invasive change that will break things for many people so I am not sure we can merge it. ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

…

On Friday, November 23, 2018 6:41 PM, brubbel ***@***.***> wrote: code readability. It is highly confusing to understand the code flow for the Node.server attribute, as the server is actually an "InternalSession (server)" instance. Replaced all ocurrences of node.server by node.isession. There is a fallback property which redirects the server attribute to node.isession, with a one-shot deprecation warning. --------------------------------------------------------------- You can view, comment on, or merge this pull request online at: #754 Commit Summary - code readability: server->isession for all InternalSession server references - show deprecation message for node.server calls. File Changes - M [opcua/common/copy_node.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-0) (12) - M [opcua/common/instantiate.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-1) (18) - M [opcua/common/manage_nodes.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-2) (30) - M [opcua/common/methods.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-3) (6) - M [opcua/common/node.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-4) (49) - M [opcua/common/shortcuts.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-5) (40) - M [opcua/common/subscription.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-6) (26) - M [opcua/common/type_dictionary_buider.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-7) (2) - M [opcua/server/standard_address_space/standard_address_space.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-8) (18) - M [opcua/tools.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-9) (6) - M [tests/tests_unit.py](https://github.com/FreeOpcUa/python-opcua/pull/754/files#diff-10) (1) Patch Links: - https://github.com/FreeOpcUa/python-opcua/pull/754.patch - https://github.com/FreeOpcUa/python-opcua/pull/754.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, [view it on GitHub](#754), or [mute the thread](https://github.com/notifications/unsubscribe-auth/ACcfzgHlm5ZH6EDrU21f-aZt8uwyjsdNks5uyDNGgaJpZM4Yw-2v).

[brubbel]

Yes, I did not thought about that.
But server is also confusing, because server can be an InternalSession or an UaClient instance. Neither of which is a Server which many people (including myself) will suppose that this attribute refers to.

You are right about the name isession, maybe it is better to find a common basis for InternalSession and UaClient.

activeSession for example could point to a server or client session.
At server side, InternalSession could inherit from activeSession base class.
At client side, the UaClient could inherit from activeSession ABC.

For me, it was a useful insight that a Node instance is related to a session, rather than to a server.
Nothing will be broken, if there is still a property that redirects the node.server to the activeSession attribute.

e.g. (isession then becomes activeSession of course)

python-opcua/opcua/common/node.py

Lines 54 to 57 in b8c727e

	@property
	def server(self):
	warnings.warn("Node.server attribute is deprecated. Use isession instead", DeprecationWarning)
	return self.isession

Also, activeSession would nicely blend with the "activate_session" method in UaClient.py.

[brubbel]

proposed scheme:

[brubbel]

Settled for session_server, because it was already used here:

python-opcua/opcua/common/type_dictionary_buider.py

Lines 121 to 130 in 9357c99

	class DataTypeDictionaryBuilder:
	def __init__(self, server, idx, idx_name, dict_name):
	self._server = server
	self._session_server = server.get_root_node().server
	self._idx = idx
	# Risk of bugs using a fixed number without checking
	self._id_counter = 8000
	self.dict_id = self._add_dictionary(dict_name)
	self._type_dictionary = OPCTypeDictionaryBuilder(idx_name)

Retained a property 'server' linking session_server for backwards compatibility.
Up to you to decide to merge or not 👍