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.

Sign up for GitHub

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

Jump to bottom

Make Sync Client and Server reusable #1503

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Make Sync Client and Server reusable #1503

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b80ffac
Refactor sync.client.conncect & sync.server.start
miki5799 Nov 23, 2023
dff554a
Merge branch 'master' into sync-reusable-client-#1364
miki5799 Dec 11, 2023
5846fea
Include Fork note in setup.py
miki5799 Dec 11, 2023
8dab330
Updated connect_sessionless and connect_socket
miki5799 Dec 12, 2023
fa02bab
Merge remote-tracking branch 'origin' into sync-reusable-client-1364
miki5799 Dec 20, 2023
7be113d
Revert "Include Fork note in setup.py"
miki5799 Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions asyncua/sync.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,12 @@ def application_uri(self):
def application_uri(self, value):
self.aio_obj.application_uri = value

@syncmethod
def connect(self) -> None:
pass
if not self.tloop.is_alive():
self.tloop = ThreadLoop()
self.tloop.start()
self.close_tloop = True
self.tloop.post(self.aio_obj.connect())
Copy link
Member

Choose a reason for hiding this comment

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

we cannot have the same code several places. At least put that code in a method.
also if you want to delay threadloop initializatin, you might need to do it at several places not only here... there is a connect_sessionless method for example and maybe others...


def disconnect(self) -> None:
try:
Expand Down Expand Up @@ -592,9 +595,12 @@ def register_namespace(self, url):
def get_namespace_array(self):
pass

@syncmethod
def start(self):
pass
if not self.tloop.is_alive():
self.tloop = ThreadLoop()
self.tloop.start()
self.close_tloop = True
self.tloop.post(self.aio_obj.start())
Copy link
Member

Choose a reason for hiding this comment

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

arrg same code again... Making that class re-usable might not be a good idea. there is probably a reason, threading.Thread is not reusable...

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your time and effort, but could you clarify further, since I'm not quite sure in what way initializing a new Thread or ThreadLoop could pose a problem in this context.

Copy link
Member

Choose a reason for hiding this comment

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

I was simply refering to the fact the threading.Thread in the python standard library does not allow re-use. We copied their API when creating that class. We can make that class reusable, but I see your code over is not really clean, so maybe it is hard to solve that issue in a safe way...


def stop(self):
self.tloop.post(self.aio_obj.stop())
Expand Down