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

Channels Retirement #7669

Closed
11 of 13 tasks
drew2a opened this issue Nov 6, 2023 · 12 comments
Closed
11 of 13 tasks

Channels Retirement #7669

drew2a opened this issue Nov 6, 2023 · 12 comments
Assignees
Milestone

Comments

@drew2a
Copy link
Contributor

drew2a commented Nov 6, 2023

Channels Retirement Plan:

Core Side:

  • Flatten metadata.db structure (completed by @egbertbouman)
  • Streamline RemoteQueryCommunity functionality. Eliminate all remote query API, preserving only the metadata retrieval mechanism. (completed by @egbertbouman)
  • Transfer the search engine to the PopularityCommunity (completed by @egbertbouman)
  • Shift endpoints for the PopularPage to MetadataStoreComponent (completed by @egbertbouman)
  • Transition metadata.db to the DatabaseComponent
  • Eliminate the GigachannelManagerComponent, GigachannelCommunityComponent (completed by @egbertbouman)
  • Rename PopularityCommunity to ContentDiscoveryCommunity
  • Merge DatabaseComponent and MetadataStoreComponent (?)

Data Side (migration):

  • Remove channel entities from the DB (?)

GUI Side:

  • Revise the main UI
  • Revise the secondary UI (debug, settings etc)
  • Adapt existing widgets to Core modifications (completed by @egbertbouman)
  • Overhaul widgets, dialogs, and models

Post-implementation, users will encounter an updated GUI:
image

On the core side, the PopularityCommunity will inherit the search feature from the ChannelCommunity.

@egbertbouman
Copy link
Member

Just in case anyone is curious, I have a branch here. Please bear in mind that this branch was meant to see if we could easily remove the channels, and shouldn't be considered stable. It took about 3-4 days to make this, while losing 7500 LoC.

@drew2a drew2a changed the title [Draft] Channels Retirement Channels Retirement Nov 9, 2023
@qstokkink
Copy link
Contributor

@egbertbouman In order to get a PR out of your branch, it seems like these are the remaining TODOs:

  1. Rebase to Tribler/main.
  2. Remove commented-out code.
  3. Restore tests for torrent_metadata.py.

Is that correct?

By the way, I'm not trying to solve ALL the bullet points listed in O.P. in a single PR. I'll leave the rest for (a) follow up PR(s).

@egbertbouman
Copy link
Member

Sounds right, although I'm not sure how many of the tests you can restore from the original test_torrent_metadata.py since ChannelMetadata/CHANNEL_TORRENT was removed (all that's left is TorrentMetadata/REGULAR_TORRENT). Having said that, there are probably some things untested for TorrentMetadata. The serialization stuff is getting tested in test_store.py.

One thing to note (for potential future PRs) is that I basically didn't touch the database. The main db table is still called ChannelNode (I added TorrentMetadata._table_ = "ChannelNode"), and all the columns from previous db versions will still be there.

@qstokkink
Copy link
Contributor

qstokkink commented Nov 27, 2023

@egbertbouman Cool, thanks 👍 That sounds like it should be a relatively quick* job then ("famous last words"). I'll get started on this now then.

*EDIT: 45 minutes of merge conflicts 🙂

@drew2a drew2a removed their assignment Nov 27, 2023
@drew2a drew2a modified the milestones: 7.15.0, 7.14.0 Nov 30, 2023
@qstokkink
Copy link
Contributor

The initial cut has now been merged in #7726. I'll move on to the (hopefully) smaller points now.

@qstokkink
Copy link
Contributor

qstokkink commented Nov 30, 2023

The next step will be to merge PopularityCommunity, RemoteQueryCommunity and VersionCommunityMixin. I guess we can rename it to ContentDiscoveryCommunity as posted in O.P.

After this I'll hand off this issue to @kozlovsky to merge the databases into one component.

@qstokkink
Copy link
Contributor

The PopularityCommunity has now been renamed to ContentDiscoveryCommunity. The multiple inheritance structure has been collapsed into a single community, all LOC now have test coverage, and some dead code was removed.

I'll start spending some time on doing "aftercare" for the two big PRs (in the form of fixing bugs).

The remaining "metadatastore -> Tribler database" refactoring(s) will remain pending until our database expert @kozlovsky comes back from vacation.

@qstokkink
Copy link
Contributor

qstokkink commented Dec 11, 2023

The "search" REST endpoint is part of metadata_store, which is being moved as part of this issue. If I start working on #7632 (comment) right now, this would lead to nasty merge conflicts. To avoid that, a small change to the roll-out plans: I'll perform the physical move of the files in metadata_store first.

After the move, the "metadatastore -> Tribler database" integration will still need to happen but it won't interfere too much with the other work because the relevant files will have been moved to a stable location.

Detailed plan

Main source moves (relative to the src/tribler/core/components folder):

  • metadata_store/category_filter -> database/category_filter
  • metadata_store/db -> database/db
  • metadata_store/restapi/metadata_endpoint.py -> database/restapi/database_endpoint.py
  • metadata_store/restapi/metadata_endpoint_base.py -> database/restapi/database_endpoint.py
  • metadata_store/restapi/metadata_schema.py:RemoteQueryParameters -> content_discovery/restapi/schema.py:RemoteQueryParameters
  • metadata_store/restapi/metadata_schema.py:{!RemoteQueryParameters} -> database/restapi/schema.py
  • metadata_store/restapi/search_endpoint.py:SearchEndpoint.completions -> database/restapi/database_endpoint.py:DatabaseEndpoint.completions
  • metadata_store/restapi/search_endpoint.py:SearchEndpoint.local_search -> database/restapi/database_endpoint.py:DatabaseEndpoint.search
  • metadata_store/restapi/search_endpoint.py:SearchEndpoint.remote_search -> content_discovery/restapi/search_endpoint.py:SearchEndpoint.search

Unit tests should be moved to the appropriate locations. Components and their settings should be merged into the appropriate components. The contents utils.py should be moved into the test configurations (if they are still used - this is probably dead code).

@qstokkink
Copy link
Contributor

With the merge of #7773, I will (try to again) partially pull out of this issue to focus on #7632 (comment)

As I laid out before, I'll wait for @kozlovsky to come back from vacation to make sure that the final merge of the TriblerDatabase and MetadataStore happens successfully. After that merge, this isssue is resolved.

@qstokkink
Copy link
Contributor

qstokkink commented Sep 19, 2024

@egbertbouman when you're back from vacation: if the upgrader script is now using explicit tables anyway (as proposed in #8163) it might be easier to do the final database file merge before the 8.0 release and handle this in the 7.14 -> 8.0 upgrade. This avoids a secondary method to upgrade from 8.0 format to 8.1 (or whatever version has the merge).

That said, this will be a lot of work again and it might cause instability just before the shiny new 8.0 release.

I think I'm mildly in favor of getting this done before the 8.0 release. What do you think?

@qstokkink qstokkink added this to the 8.1.0 milestone Sep 23, 2024
@qstokkink
Copy link
Contributor

[Offline discussion summary] Do we even want everything in the same database? Aesthetically, one database file is certainly more appealing. However, from a development perspective, everything is worse: we have to create and maintain complex database migrations and one Tribler experimental module can corrupt the database for all other modules. Current sentiment is to just keep separate database files.

@egbertbouman
Copy link
Member

@qstokkink Agreed, let's close this.

@qstokkink qstokkink modified the milestones: 8.1.0, 8.0.6 Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants