Moving away from the global context #1841

New Issue

2021-09-05T16:17:35+02:00

g1itch commented

2021-09-05 16:17:35 +02:00

(Migrated from github.com)

Hello!

Your comment in #1839 triggered a huge reflection in my brain regarding the architecture of threads in the main package. I suddenly realized that they have the same birth injury as a bitmessageqt app: global context. I started a branch incapsulation where I'm going to address the issue in the singeWorker as an example. You cannot just take the current code and write a test case which start the singeWorker thread isolated from the rest ones, because it uses SQL, Inventory, queues and the shared objects...

In this ticket I'm going to describe the solutions I already found and formulated as they arrive, so we could continue the discussion, started in #1828.

The points so far:

if the thread has it's own queue, where it takes commands, init it with the queue
the same is applicable to other objects such as Inventory, BMConfigParser, etc.
worker has nothing to do with invQueue, Inventory can feed it itself
I'm prematurely introducing a new entity, MsgBox, the base class for Inbox, Outbox, etc. The worker needs it's method ~ get_by_status() for the queries like this. The implementation is not have to be tied to the SQLThread.
once all the objects passed directly to their consumers, get rid of the Singleton anti-pattern.

The minor issues can be addressed there:

use something like getattr(self, 'handle_{}'.format(command)) instead of the if: else: ladder
try handle the exceptions where they were rised, reducing the duplicates
move all assembly code into the protocol or it's imported submodule (e.g. fe691a5) and cover by tests

the latter will help removing all the pylint comments. You will also see why I hate those tiny formatting changes: I see most of those blocks removed or moved into another modules (e.g. cf8c3ac) but writing code takes time ):

Hello! Your comment in #1839 triggered a huge reflection in my brain regarding the architecture of threads in the main package. I suddenly realized that they have the same birth injury as a `bitmessageqt` app: global context. I started a branch [incapsulation](https://github.com/g1itch/PyBitmessage/tree/incapsulation) where I'm going to address the issue in the `singeWorker` as an example. You cannot just take the current code and write a test case which start the `singeWorker` thread isolated from the rest ones, because it uses SQL, Inventory, queues and the shared objects... In this ticket I'm going to describe the solutions I already found and formulated as they arrive, so we could continue the discussion, started in #1828. The points so far: - if the thread has it's own queue, where it takes commands, [init it with the queue](https://github.com/g1itch/PyBitmessage/commit/0978d7789d386c17f167ed9fa8f67342240c3341#diff-733625a72542a829f0ec5bd93fded7b7451f5c7f1f9a59997bf4d18ec4471a10R51) - the same is applicable to other objects such as Inventory, BMConfigParser, etc. - worker has nothing to do with `invQueue`, Inventory [can feed it itself](https://github.com/g1itch/PyBitmessage/commit/0978d7789d386c17f167ed9fa8f67342240c3341#diff-6cf2ada67d0ea7cbf66242ffe357fda3064d3312dad11718809bf7f5fa7eb07bR47) - I'm prematurely introducing a new entity, `MsgBox`, the base class for Inbox, Outbox, etc. The worker needs it's method ~ `get_by_status()` for the queries like [this](https://github.com/Bitmessage/PyBitmessage/blob/v0.6/src/class_singleWorker.py#L725). The implementation is not have to be tied to the `SQLThread`. - once all the objects passed directly to their consumers, **get rid of the Singleton anti-pattern**. The minor issues can be addressed there: - use something like `getattr(self, 'handle_{}'.format(command))` instead of the [if: else: ladder](https://github.com/Bitmessage/PyBitmessage/blob/v0.6/src/class_singleWorker.py#L149) - try handle the exceptions where they were rised, reducing the duplicates - move all assembly code into the `protocol` or it's imported submodule (e.g. fe691a5) and cover by tests the latter will help removing all the pylint comments. You will also see why I hate those tiny formatting changes: I see most of those blocks removed or moved into another modules (e.g. cf8c3ac) but writing code takes time ):

This repo is archived. You cannot comment on issues.