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.

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

Loose coupling and sane folders structure #4953

Closed
ichorid opened this issue Nov 19, 2019 · 8 comments
Closed

Loose coupling and sane folders structure #4953

ichorid opened this issue Nov 19, 2019 · 8 comments

Comments

@ichorid
Copy link
Contributor

ichorid commented Nov 19, 2019

The Mess

Our source code directories structure is a mess. As is our import/modules structures (I don't even want to start on the topic of inconsistent dir names capitalization...). For some things, like REST endpoints, sources are grouped by function:

Tribler/Core/Modules/restapi/*_endpoint.py

For other things by component:

Tribler/Core/Modules/MetadataStore/*
Tribler/Core/CreditMininig/*

(Note the inconsistent placement of modules!)

Another problem is the Config. Every component depends on the base Config object. When anyone wants to change something there, they have to add get_*/set_* methods directly into the Config class. As a result, the Config became a garbage dump. Also, it creates tight coupling and prevents submodules (like Anydex) from getting really independent (which defies the whole idea of having a submodule).

The Solution

We can make Tribler more modularized, and component less dependent on each other by changing the source folders structure and moving component-specific sections of the Config class into the corresponding modules/components.

Directories

The directory structure could look like this:

Tribler/Core/Session.py
  ... (and friends)
Tribler/Core/Modules/
Tribler/Core/Modules/MetadataStore - (component dir)
Tribler/Core/Modules/MetadataStore/restapi/channels_endpoint.py - (channels endpoint)
Tribler/Core/Modules/MetadataStore/config.py (sub-config)
...
Tribler/Core/Modules/CreditMining
Tribler/Core/Modules/Anydex
Tribler/Core/Modules/TorrentChecker
Tribler/Core/Modules/ ... (etc)

Subconfigs

Subconfigs will enhance the root Config class as member objects, like this:

class TriblerConfig(object):

    def __init__(self, config=None):
        ...
        self.metadata_store_config = MetadataStore.config(self.config)

After joining LaunchMany with Session, we'll be able to loosen the coupling between Tribler components and make their initialisation sequences unified and straightforward, like this:

class Session(object):
    def start(config=None):
        if self.config.metadata_store_config.enabled:
            self.mds = MetadataStore(config=self.config.metadata_store_config)

In this scheme, ComponentConfig objects could look like this:

class ComponentConfig(object):
        def __init__(self, config):
            self._config = config
        
        @property
        def enabled():
             return self._config["enabled"]
        
        @property.setter
        def enabled(val):
             self._config["enabled"] = val

The root Config object will be the sole thing responsible for writing/reading stuff to disk. The downstream ComponentConfig objects will propagate the changes to it by the virtue of dict mutability.

These changes will make the code structure much more straightforward, clear, and manageable.

@qstokkink
Copy link
Contributor

I agree, much of the Tribler code was written when camelCase was still acceptable in Python. Let's also make sure we adhere to modern standards by using lowercase module names.

@ichorid
Copy link
Contributor Author

ichorid commented Nov 19, 2019

@qstokkink , could you please compile a list of modern Python standards that we should adhere to during coding/refactoring? That should help the effort.

@qstokkink
Copy link
Contributor

@ichorid
Copy link
Contributor Author

ichorid commented Dec 18, 2019

After discussing the thing with @qstokkink and @xoriole , we came up with the following folders structure:

build
... (non-source stuff like builders, docs, etc)
doc
src/pyipv8
src/triblergui
src/triblercore
src/triblercommon    (<-this contains things shared by Core and GUI)
src/run_tribler.py
src/run_tunnel_helper.py  (various runners)
src/tests/triblercore
src/tests/triblergui

This structure solves several problems:

  • tribler* dirs constitue separate packages that can be easily built and imported with pip;
  • tests are out of package dirs, so these will not be packaged in case of pip builds;
  • toplevel sturcture of src, docs, etc. is clear to follow for the reader;

In the triblercore dir, components will be organised in the following way:

core
core/session.py
modules
modules/<modulename>
modules/<modulename>/rest_endpoint.py
modules/<modulename>/community.py
utils
utils/urlutils.py

i.e. each Tribler component, such as Credit Mining or Metadata Store will get a separate directory. This directory will contain everything related to this module: IPv8 community, REST endpointss, config specifications, etc. This change paves the way to implementing a plugins system in Tribler.

@ichorid
Copy link
Contributor Author

ichorid commented Dec 23, 2019

After discussion with @devos50 we decided that it will be better to include the tests besides the modules themselves in the code. This way, it will force the programmer to immediately think about the tests as an integral part of the code, and ease the code handling.

@ichorid
Copy link
Contributor Author

ichorid commented Jan 4, 2020

Now that the folders structure is changed, we can start making Tribler modules use "lazy loading" strategy and being more independent from each other. Also, we can explore asynchronous start-up.

@ichorid
Copy link
Contributor Author

ichorid commented Jun 30, 2021

Tribler source code now is at the point where we can completely remove the Session mediator-God-object in favor of a single procedure looking like this

async def start_core(a,b):
   foo = ComponentFoo(a,b)
   await foo.start()
   bar = ComponentBar(foo, b)
   await bar.start()

   await shutdown_future

   await bar.shutdown()
   await foo.shutdown()

Furthermore, we can use the DependencyInjector framework to initialize components on-demand. This change will finally bring us loose coupling at the top level of Tribler Core, significantly simplifying all of the top-level architecture and tests.

The next step after removing Session will be splitting the run_core procedure into a bunch of smaller Tasks, that will be able to start and shutdown asynchronously, thus speeding up Tribler initialization.

@drew2a drew2a added this to the Backlog milestone Sep 15, 2021
@ichorid
Copy link
Contributor Author

ichorid commented Sep 24, 2021

Solved by #6335

@ichorid ichorid closed this as completed Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

3 participants