Skip to content

No(Only One) Thread in ClientArch#1230

Open
fernandohds564 wants to merge 9 commits intomasterfrom
dev/enh/clientarch-nothread
Open

No(Only One) Thread in ClientArch#1230
fernandohds564 wants to merge 9 commits intomasterfrom
dev/enh/clientarch-nothread

Conversation

@fernandohds564
Copy link
Copy Markdown
Contributor

@fernandohds564 fernandohds564 commented Mar 24, 2026

When we updated the clientarch subpackage to use asynchronous requests to the server under the hood, we implemented it so that all async methods run in a separate thread. This was initially thought to be necessary to avoid conflicts in environments where an event loop is already running, such as Jupyter notebooks.

However, this approach introduces several issues. Exceptions raised in that separate thread cannot be caught by the end user, and errors do not interrupt the execution flow as expected. In addition, there are cases where the thread silently dies, causing user calls to hang indefinitely.

It turns out there is a better way to run async methods under the hood without exposing async in the public API. This PR attempts to implement that approach. Most of the implementation was done by GitHub Copilot, with a few manual adjustments on my part. One of those changes is the introduction of a dependency on the nest_asyncio package.

Since the nest_asyncio package was archived, we implemented again a solution based on running a different event loop in a new thread. However, this thread is long lived and all tasks are submitted using asyncio.run_coroutine_threadsafe function. This guarantees that exceptions will be passed to the end user and that there will be no infinite hangs.

This last version of the code was done with the help of Google AI Mode.

This PR also solves a bug related on how to properly use aiohttp. Now the json parsing of the response happens within a async with for the get resquest. Thanks to @anacso17 for helping with this debugging.

The bug fix above, made it impossible to keep the old method to check for connection in the ClientArchiver.connected property, since we need to convert the response, either to json or to text within the async with context manager. For this reason, the option return_json was removed. Now all requests will return data in json format, falling back to pure text data when json conversion fails, which is the case of the ClientArchiver.connected method.

This PR also fixes the ClientArchiver.login() method that was not working in computers where the server is defined by its IP address, instead of its hostname (control room computers).

This version of the code was tested in our conda environments and at the control room.

This code was done with the help of Google AI Mode.

Since the nest_asyncio package was archived,
we implemented again a solution based on
running a different event loop in a new thread
however, this thread is long lived and all tasks
are submitted using asyncio.run_coroutine_threadsafe
function. Which guarantees that exceptions will
be passed to the end user and that there will be
no infinite hangs.
@fernandohds564 fernandohds564 requested a review from anacso17 March 25, 2026 14:35
@fernandohds564 fernandohds564 changed the title No threads in ClientArch ~~No~~ One thread~~s~~ in ClientArch Mar 25, 2026
@fernandohds564 fernandohds564 changed the title ~~No~~ One thread~~s~~ in ClientArch No(Only One) Thread in ClientArch Mar 25, 2026
Simplifies request logic by removing the need for a return_json flag,
centralizing response deserialization, and improving error handling for
timeouts and payload issues. Enhances code clarity by isolating JSON/text
parsing and standardizing return values. Prepares for more robust
integration and debugging in asynchronous workflows.
@fernandohds564 fernandohds564 requested a review from anacso17 April 1, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants