-
Notifications
You must be signed in to change notification settings - Fork 451
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
Moved metadata_store into database and content_discovery #7773
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
9 changes: 9 additions & 0 deletions
9
src/tribler/core/components/content_discovery/restapi/schema.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
from marshmallow.fields import Integer, String | ||
|
||
from tribler.core.components.database.restapi.schema import MetadataParameters | ||
|
||
|
||
class RemoteQueryParameters(MetadataParameters): | ||
uuid = String() | ||
channel_pk = String(description='Channel to query, must also define origin_id') | ||
origin_id = Integer(default=None, description='Peer id to query, must also define channel_pk') |
71 changes: 71 additions & 0 deletions
71
src/tribler/core/components/content_discovery/restapi/search_endpoint.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
from binascii import hexlify, unhexlify | ||
|
||
from aiohttp import web | ||
from aiohttp_apispec import docs, querystring_schema | ||
from marshmallow.fields import List, String | ||
|
||
from ipv8.REST.schema import schema | ||
from tribler.core.components.content_discovery.community.content_discovery_community import ContentDiscoveryCommunity | ||
from tribler.core.components.content_discovery.restapi.schema import RemoteQueryParameters | ||
from tribler.core.components.restapi.rest.rest_endpoint import HTTP_BAD_REQUEST, MAX_REQUEST_SIZE, RESTEndpoint, \ | ||
RESTResponse | ||
from tribler.core.utilities.utilities import froze_it | ||
|
||
|
||
@froze_it | ||
class SearchEndpoint(RESTEndpoint): | ||
""" | ||
This endpoint is responsible for searching in channels and torrents present in the local Tribler database. | ||
""" | ||
path = '/search' | ||
|
||
def __init__(self, | ||
popularity_community: ContentDiscoveryCommunity, | ||
middlewares=(), | ||
client_max_size=MAX_REQUEST_SIZE): | ||
super().__init__(middlewares, client_max_size) | ||
self.popularity_community = popularity_community | ||
|
||
def setup_routes(self): | ||
self.app.add_routes([web.put('/remote', self.remote_search)]) | ||
|
||
@classmethod | ||
def sanitize_parameters(cls, parameters): | ||
sanitized = dict(parameters) | ||
if "max_rowid" in parameters: | ||
sanitized["max_rowid"] = int(parameters["max_rowid"]) | ||
if "channel_pk" in parameters: | ||
sanitized["channel_pk"] = unhexlify(parameters["channel_pk"]) | ||
if "origin_id" in parameters: | ||
sanitized["origin_id"] = int(parameters["origin_id"]) | ||
return sanitized | ||
|
||
|
||
@docs( | ||
tags=['Metadata'], | ||
summary="Perform a search for a given query.", | ||
responses={200: { | ||
'schema': schema(RemoteSearchResponse={'request_uuid': String(), 'peers': List(String())})}, | ||
"examples": { | ||
'Success': { | ||
"request_uuid": "268560c0-3f28-4e6e-9d85-d5ccb0269693", | ||
"peers": ["50e9a2ce646c373985a8e827e328830e053025c6", "107c84e5d9636c17b46c88c3ddb54842d80081b0"] | ||
} | ||
} | ||
}, | ||
) | ||
@querystring_schema(RemoteQueryParameters) | ||
async def remote_search(self, request): | ||
self._logger.info('Create remote search request') | ||
# Query remote results from the GigaChannel Community. | ||
# Results are returned over the Events endpoint. | ||
try: | ||
sanitized = self.sanitize_parameters(request.query) | ||
except (ValueError, KeyError) as e: | ||
return RESTResponse({"error": f"Error processing request parameters: {e}"}, status=HTTP_BAD_REQUEST) | ||
self._logger.info(f'Parameters: {sanitized}') | ||
|
||
request_uuid, peers_list = self.popularity_community.send_search_request(**sanitized) | ||
peers_mid_list = [hexlify(p.mid).decode() for p in peers_list] | ||
|
||
return RESTResponse({"request_uuid": str(request_uuid), "peers": peers_mid_list}) |
File renamed without changes.
64 changes: 64 additions & 0 deletions
64
src/tribler/core/components/content_discovery/restapi/tests/test_search_endpoint.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
import uuid | ||
from unittest.mock import Mock | ||
|
||
import pytest | ||
|
||
from tribler.core.components.content_discovery.restapi.search_endpoint import SearchEndpoint | ||
from tribler.core.components.restapi.rest.base_api_test import do_request | ||
from tribler.core.utilities.unicode import hexlify | ||
|
||
|
||
@pytest.fixture(name="mock_content_discovery_community") | ||
def fixture_mock_content_discovery_community(): | ||
return Mock() | ||
|
||
|
||
@pytest.fixture(name="endpoint") | ||
def fixture_endpoint(mock_content_discovery_community): | ||
return SearchEndpoint(mock_content_discovery_community) | ||
|
||
|
||
async def test_create_remote_search_request(rest_api, mock_content_discovery_community): | ||
""" | ||
Test that remote search call is sent on a REST API search request | ||
""" | ||
sent = {} | ||
peers = [] | ||
request_uuid = uuid.uuid4() | ||
|
||
def mock_send(**kwargs): | ||
sent.update(kwargs) | ||
return request_uuid, peers | ||
|
||
# Test querying for keywords | ||
mock_content_discovery_community.send_search_request = mock_send | ||
search_txt = "foo" | ||
await do_request( | ||
rest_api, | ||
f'search/remote?txt_filter={search_txt}&max_rowid=1', | ||
request_type="PUT", | ||
expected_code=200, | ||
expected_json={"request_uuid": str(request_uuid), "peers": peers}, | ||
) | ||
assert sent['txt_filter'] == search_txt | ||
sent.clear() | ||
|
||
# Test querying channel data by public key, e.g. for channel preview purposes | ||
channel_pk = "ff" | ||
await do_request( | ||
rest_api, f'search/remote?channel_pk={channel_pk}&metadata_type=torrent', request_type="PUT", expected_code=200 | ||
) | ||
assert hexlify(sent['channel_pk']) == channel_pk | ||
|
||
|
||
async def test_create_remote_search_request_illegal(rest_api): | ||
""" | ||
Test that remote search call is sent on a REST API search request | ||
""" | ||
response = await do_request( | ||
rest_api, | ||
'search/remote?origin_id=a', | ||
request_type="PUT", | ||
expected_code=400 | ||
) | ||
assert "error" in response |
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
4 changes: 2 additions & 2 deletions
4
...re/category_filter/tests/test_category.py → ...se/category_filter/tests/test_category.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 2 additions & 2 deletions
4
...tegory_filter/tests/test_family_filter.py → ...tegory_filter/tests/test_family_filter.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 2 additions & 2 deletions
4
...tegory_filter/tests/test_init_category.py → ...tegory_filter/tests/test_init_category.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
48 changes: 46 additions & 2 deletions
48
src/tribler/core/components/database/database_component.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,67 @@ | ||
from tribler.core import notifications | ||
from tribler.core.components.component import Component | ||
from tribler.core.components.database.db.store import MetadataStore | ||
from tribler.core.components.database.db.tribler_database import TriblerDatabase | ||
from tribler.core.components.key.key_component import KeyComponent | ||
from tribler.core.components.knowledge.rules.knowledge_rules_processor import KnowledgeRulesProcessor | ||
from tribler.core.utilities.simpledefs import STATEDIR_DB_DIR | ||
|
||
|
||
class DatabaseComponent(Component): | ||
tribler_should_stop_on_component_error = True | ||
|
||
db: TriblerDatabase = None | ||
mds: MetadataStore = None # TODO: legacy, should be merged into ``db`` | ||
|
||
async def run(self): | ||
await super().run() | ||
|
||
db_path = self.session.config.state_dir / STATEDIR_DB_DIR / "tribler.db" | ||
if self.session.config.gui_test_mode: | ||
config = self.session.config | ||
db_path = config.state_dir / STATEDIR_DB_DIR / "tribler.db" | ||
if config.gui_test_mode: | ||
db_path = ":memory:" | ||
|
||
self.db = TriblerDatabase(str(db_path)) | ||
|
||
# TODO: merge the code below into the TriblerDatabase | ||
|
||
channels_dir = config.chant.get_path_as_absolute('channels_dir', config.state_dir) | ||
chant_testnet = config.general.testnet or config.chant.testnet | ||
|
||
metadata_db_name = 'metadata.db' | ||
if chant_testnet: | ||
metadata_db_name = 'metadata_testnet.db' | ||
elif config.gui_test_mode: # Avoid interfering with the main database in test mode | ||
# Note we don't use in-memory database in core test mode, because MDS uses threads, | ||
# and SQLite creates a different in-memory DB for each connection by default. | ||
# To change this behaviour, we have to use url-based SQLite initialization syntax, | ||
# which is not supported by PonyORM yet. | ||
metadata_db_name = 'metadata_gui_test.db' | ||
|
||
database_path = config.state_dir / STATEDIR_DB_DIR / metadata_db_name | ||
|
||
# Make sure that we start with a clean metadata database when in GUI mode every time. | ||
if config.gui_test_mode and database_path.exists(): | ||
self.logger.info("Wiping metadata database in GUI test mode") | ||
database_path.unlink(missing_ok=True) | ||
|
||
key_component = await self.require_component(KeyComponent) | ||
|
||
metadata_store = MetadataStore( | ||
database_path, | ||
channels_dir, | ||
key_component.primary_key, | ||
notifier=self.session.notifier, | ||
disable_sync=config.gui_test_mode, | ||
tag_processor_version=KnowledgeRulesProcessor.version | ||
) | ||
self.mds = metadata_store | ||
self.session.notifier.add_observer(notifications.torrent_metadata_added, | ||
metadata_store.TorrentMetadata.add_ffa_from_dict) | ||
|
||
async def shutdown(self): | ||
await super().shutdown() | ||
if self.db: | ||
self.db.shutdown() | ||
if self.mds: | ||
self.mds.shutdown() |
File renamed without changes.
File renamed without changes.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a TODO for the next PR, or is it a forgotten TODO for the current PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for the next PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the developers and I in this group consider it bad, or let's say "not good," practice to merge changes with TODOs into the main branch.
I'm not stopping you from doing this by not giving acceptance to the PR. I'm just highlighting that there are many points of view on this practice, some of which consider it to be a bad practice that leads to code quality degradation.
https://www.reddit.com/r/learnprogramming/comments/r25bj7/are_is_committing_code_to_github_with_todos_bad/?rdt=46532
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with them. However, our current linters are configured to allow this. Notably, I did not have to disable any warnings to pass our PR checks.
Once we fix #1722 we can adopt a more strict stance on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying that if you understand there is a bad practice, but there are no configured linter issues, then it is okay for you to commit and merge the code despite its quality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though I disagree with the practice personally, this is the approach that the team has chosen and I adapt my workflow to it.
If I were the only developer, I would make make own standards and conform to what I would like to see. Sadly, I am not the only developer and I have to conform to the existing rules and linter settings.