Open kozlovsky opened 2 years ago
Now I can return to Andrey's notes about the current realization of components:
For each of the components, there are two types of objects:
- Implementation
- Interface
My main notes for interfaces are:
- Interfaces are unnecessary here (we can remove them)
Speaking about component objects, it is useful to separate the interface and the implementation of a component for the following reasons:
Speaking about component modules, you are correct that we can put an interface and the main implementation of the same component into the same module. But having different modules for interface and implementation is helpful for two reasons:
- Interfaces are not what they seem (technically, they looks more like Facades (https://en.wikipedia.org/wiki/Facade_pattern))
As I already wrote above, I disagree with this. In the current implementation, interfaces describe component fields in a strongly typed way. Inside your component, you can write (await self.use(LibtorrentComponent)).download_manager
, and PyCharm will correctly identify that the result's type is a DownloadManager
instance.
Let me compare the current way how components work with the previous iteration of the component architecture.
On the previous iteration, components indeed do not have interfaces. Each component had a single value which was provided by that component:
class TunnelsComponent(Component):
role = TUNNELS_COMMUNITY
...
async def run(self, mediator):
await super().run(mediator)
config = mediator.config
ipv8 = await self.use(mediator, IPV8_SERVICE)
bandwidth_community = await self.use(mediator, BANDWIDTH_ACCOUNTING_COMMUNITY)
peer = await self.use(mediator, MY_PEER)
dht_community = await self.use(mediator, DHT_DISCOVERY_COMMUNITY)
download_manager = await self.use(mediator, DOWNLOAD_MANAGER)
bootstrapper = await self.use(mediator, IPV8_BOOTSTRAPPER)
rest_manager = await self.use(mediator, REST_MANAGER)
In my opinion, this approach had the following drawbacks:
ipv8
or peer
here.IPV8_SERVICE
, MY_PEER
, and DHT_DISCOVERY_COMMUNITY
to pass three related values to TunnelsComponent.With component interfaces, we now have typed values, and a single component can hold more than one value. Now the same code looks like this:
class TunnelsComponentImp(TunnelsComponent):
async def run(self):
await self.use(ReporterComponent, required=False)
config = self.session.config
ipv8_component = await self.use(Ipv8Component)
ipv8 = ipv8_component.ipv8
peer = ipv8_component.peer
dht_discovery_community = ipv8_component.dht_discovery_community
bandwidth_component = await self.use(BandwidthAccountingComponent, required=False)
bandwidth_community = bandwidth_component.community if bandwidth_component.enabled else None
download_component = await self.use(LibtorrentComponent, required=False)
download_manager = download_component.download_manager if download_component.enabled else None
rest_component = await self.use(RESTComponent, required=False)
rest_manager = rest_component.rest_manager if rest_component.enabled else None
It does not look more concise because the code now takes into account that some optional components can be disabled in the Tribler configuration. But you can see the following differences:
Ipv8Component
can now provide multiple values: ipv8
, peer
and dht_discovery_community
.ipv8
or peer
is.
- A part of the interface's implementation has to be moved to the tests (see comments in the example below)
As I wrote above, in my opinion, this is confusion caused by an unfortunate naming. FoobarComponentMock
class is not intended to be used in tests exclusively. Its main purpose is to be used as a dummy replacement for the component when it is disabled in Tribler configuration. I think it is better to rename such classes to something like DummyFoobarComponent
.
We have the following folder structure: https://github.com/Tribler/tribler/tree/main/src/tribler-core/tribler_core
- components
- config
- modules
- restapi
- tests
- upgrade
- utilities
Components and modules should be merged into one single folder (as new "components" is old "modules").
Eventually, we should probably merge components and modules into a single folder. But right now, we don't have a one-to-one mapping between components and files in the modules
folder.
In previous iteration of components architecture, components reside in the modules directory. The directory structure looked like this (a subset of files and subdirectories):
modules
bandwidth_accounting
bandwidth_endpoint.py
community.py
component.py
...
category_filter
category.py
... (no component.py here)
exception_handler
component.py
exception_handler.py
...
ipv8
component.py
libtorrent
download.py
download_manager.py
... (no component.py here)
metadata_store
community
component.py
gigachannel_community.py
...
manager
component.py
gigachannel_manager.py
restapi
channels_endpoint.py
metadata_endpoint.py
... (no component.py here)
component.py
config.py
...
(...other subdirectories, some of them with components, others without)
bootstrap.py
component.py (base component class)
dht_health_manager.py
...
Was this directory layout more convenient as compared to having all components in a dedicated directory? To me, it wasn't, for the following reasons:
metadata_store/component.py
, metadata_store/community/component.py
, metadata_store/manager/component.py
, etc. With this layout, it is easy to miss a component hidden in some subfolder of another component.Having component implementations in a separate folder solves these two problems:
components
implementations
bandwith_accounting.py
gigachannel.py
gigachannel_manager.py
ipv8.py
libtorrent.py
(... and so on)
But in return, you need to have one more separate directory.
Each layout has some benefits and drawbacks. So, choosing the directory layout is a trade-off. We need to discuss it and choose a less annoying layout that has the least number of drawbacks. Then we can switch to the most desired layout quickly, with a simple IDE-supported refactoring.
There are no comments for the new complex logic (base.py).
Agree with that. I need to add comments and probably some internal documentation. Probably descriptions from the current thread can be used as a start for it.
In conclusion, I think that the current implementation of components has its warts. I usually don't like to have a big number of files and directories, as well as boilerplate code. I hope we can improve the current architecture after the discussion. But we need to have a common understanding of component architecture goals, possible trade-offs, and viable alternatives. Hopefully, with the forthcoming meeting, we can achieve this.
I'm excited about the prospect of an offline all-dev-team meeting for discussing components. I'm sure that with the collective discussion, we can make the components architecture much better.
The current version of the components architecture has not emerged from an empty initial state. It was an incremental refactoring of the previous approach that you can see here.
First, I want to describe the component architecture that we have right now. The current implementation of components is trying to achieve the following goals:
Let's see how the current components implementation achieves that.
Static typing of components
Flexible configuration of Tribler
How components solve the problem of circular imports
Direct communication of components
Asynchronous initialization of components
Assembling arbitrary configurations of components