Made TorrentDef.load async and threaded

[qstokkink]

Related to #5399 (PR 2/4)

This PR moves TorrentDef.load to a thread. The fallout from this - essentially 12-line - change consists of 245 additions and 309 deletions across 21 files.

Our codecov check uncovered some dead code, which I removed (instead of writing tests for it):

• tribler/core/components/metadata_store/tests/gen_test_data.py
• tribler/core/components/metadata_store/restapi/tests/test_channels_endpoint.py (test_add_torrent_from_magnet)

[kozlovsky]

Overall looks good, some bugs need to be addressed

[kozlovsky]

Looks like an indentation bug. When the if condition is not satisfied, the dcfg local variable is not defined, and we can't refer to it in the function call.

Suggested change

	return await self.download_manager.start_download(tdef=tdef, config=dcfg)
	return await self.download_manager.start_download(tdef=tdef, config=dcfg)

[kozlovsky]

It looks like other functions from this file are also not used, and the entire file can be deleted. (not just the gen_sample_channel function)

[kozlovsky]

Note that the start_handle method is an async method that returns a Future object as a result. Here, we wait only for the start_handle method itself and not for the future it returns. If we want to wait for the future returned from the method, we should write await (await self.start_handle(download, atp)).

As I understand it, it is not necessary to wait for the future result here. But then, the question is, should we return the future from the start_handle method at all? Currently, we never await for the future result returned from the start_handle method, so it may be cleaner to just return None instead of the future.

[kozlovsky]

As I mentioned in the previous comment, we never await for the future returned from the start_handle method, not even in tests. It may be unclear for the reader of the code what the await self.start_handle(...) call actually waits for. I suggest that we remove this return statement completely to make the code a bit easier to understand.

[kozlovsky]

Previously, this code was inside db_session. PonyORM ignores nested db session in the add_torrent_to_channel method, and, as a result, each batch of 100 torrents was processed as a single transaction.

Now, the top-level db_session is removed from the current method, and each individual add_torrent_to_channel call is processed as a separate transaction. It can slow down adding a large number of torrents. Basically, torrents are not grouped into batches anymore, and the orm.commit() call does nothing, as there is no active transaction at this moment.

I suggest changing the code a bit: we can load all TorrentDef instances first and then store them in the database in a single db_session with batches:

            torrent_defs = []
            for filename in torrents_list:
                try:
                    torrent_defs.append(await TorrentDef.load(filename))
                except Exception:  # pylint: disable=W0703
                    # Have to use the broad exception clause because Py3 versions of libtorrent
                    # generate generic Exceptions
                    errors_list.append(filename)

            with db_session:
                for chunk in chunks(torrent_defs, 100):
                    for tdef in chunk:
                        self.add_torrent_to_channel(tdef)
                    orm.commit()

[qstokkink]

@kozlovsky Thanks for your review. I addressed your points pretty much one-to-one according to your suggestions. Regarding start_handle, I adopted your suggestion of returning None.

I requested your re-review: please ensure that I indeed adopted your suggestions as you intended.

[kozlovsky]

Everything looks good now!