-
Notifications
You must be signed in to change notification settings - Fork 46
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
feat: meilisearch backend #150
Changes from 10 commits
ef60540
c60f577
d2b92fc
f9d2de1
62ac4b0
b0e8109
1e4261a
4bccabb
edee7ae
e64a5bc
3a240df
2fe2800
4450a37
2073181
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ | |
# This is just a container for running tests | ||
DEBUG = True | ||
|
||
ALLOWED_HOSTS = [] | ||
ALLOWED_HOSTS = ['*'] | ||
|
||
TEMPLATES = [ | ||
{ | ||
|
@@ -99,3 +99,21 @@ | |
# https://docs.djangoproject.com/en/1.6/howto/static-files/ | ||
|
||
STATIC_URL = '/static/' | ||
################### Using ElasticSearch ################### | ||
|
||
SEARCH_ENGINE = os.getenv('SEARCH_ENGINE', 'search.elastic.ElasticSearchEngine') | ||
|
||
################### Using Meilisearch (Beta) ################### | ||
|
||
# Enable Studio search features (powered by Meilisearch) (beta, off by default) | ||
MEILISEARCH_ENABLED = False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it makes sense to have a setting called |
||
# Meilisearch URL that the python backend can use. Often points to another docker container or k8s service. | ||
MEILISEARCH_URL = os.getenv('MEILISEARCH_URL', 'http://localhost:7700') | ||
# URL that browsers (end users) can use to reach Meilisearch. Should be HTTPS in production. | ||
MEILISEARCH_PUBLIC_URL = os.getenv('MEILISEARCH_PUBLIC_URL', 'http://localhost:7700') | ||
# To support multi-tenancy, you can prefix all indexes with a common key like "sandbox7-" | ||
# and use a restricted tenant token in place of an API key, so that this Open edX instance | ||
# can only use the index(es) that start with this prefix. | ||
# See https://www.meilisearch.com/docs/learn/security/tenant_tokens | ||
MEILISEARCH_INDEX_PREFIX = os.getenv('MEILISEARCH_INDEX_PREFIX', '') | ||
MEILISEARCH_API_KEY = os.getenv('MEILISEARCH_API_KEY', 'masterKey') |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,3 +15,4 @@ Django # Web application framework | |
elasticsearch>=7.8.0,<8.0.0 | ||
edx-toggles | ||
event-tracking | ||
meilisearch==0.31.1 |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,8 @@ | ||||||
""" search business logic implementations """ | ||||||
|
||||||
from datetime import datetime | ||||||
|
||||||
import meilisearch | ||||||
from django.conf import settings | ||||||
|
||||||
from eventtracking import tracker as track | ||||||
|
@@ -158,3 +160,102 @@ def course_discovery_search(search_term=None, size=20, from_=0, field_dictionary | |||||
) | ||||||
|
||||||
return results | ||||||
|
||||||
|
||||||
def _meilisearch_auto_suggest_search_api(term, course_id, limit=30): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be a method on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removing this endpoint |
||||||
""" | ||||||
Perform an auto-suggest search using the Elasticsearch search engine. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is an "auto-suggest search"? How is it different than a regular search? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was a POC for implementation I am going to remove it |
||||||
|
||||||
Args: | ||||||
term (str): The search term. | ||||||
course_id (str): The ID of the course to filter the search results. | ||||||
limit (int, optional): The maximum number of results to return. Defaults to 30. | ||||||
|
||||||
Returns: | ||||||
list: A list of dictionaries containing the search results with 'id', 'display_name', and 'usage_key'. | ||||||
""" | ||||||
# Create a client instance for MeiliSearch | ||||||
client = meilisearch.Client(settings.MEILISEARCH_URL, settings.MEILISEARCH_API_KEY) | ||||||
|
||||||
# Define the index name | ||||||
index_name = settings.MEILISEARCH_INDEX_PREFIX + "studio_content" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is searching the studio index. It doesn't make sense. It should get the index name from the subclass, and it definitely shouldn't be searching the studio content. For courseware search, it should be |
||||||
|
||||||
# Perform the search with specified facets and filters | ||||||
results = client.index(index_name).search(term, { | ||||||
"facets": ["block_type", "tags"], | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think any code using edx-search is aware of |
||||||
"filter": [f"context_key='{course_id}'"], | ||||||
"limit": limit | ||||||
}) | ||||||
|
||||||
# Process the search hits to extract relevant fields | ||||||
results = list(map(lambda it: { | ||||||
"id": it["id"], | ||||||
"display_name": it["display_name"], | ||||||
"usage_key": it["usage_key"], | ||||||
}, results["hits"])) | ||||||
|
||||||
return results | ||||||
|
||||||
|
||||||
def _elasticsearch_auto_suggest_search_api(term, course_id, limit=30): | ||||||
""" | ||||||
Perform an auto-suggest search using either Elasticsearch or MeiliSearch based on configuration. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is an "auto-suggest search" ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be moved to the |
||||||
|
||||||
Args: | ||||||
term (str): The search term. | ||||||
course_id (str): The ID of the course to filter the search results. | ||||||
limit (int, optional): The maximum number of results to return. Defaults to 30. | ||||||
|
||||||
Returns: | ||||||
list: A list of dictionaries containing the search results with 'id', 'display_name' and 'usage_key'. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please define this using type hints rather than just describing it. |
||||||
""" | ||||||
|
||||||
# Get the search engine instance | ||||||
searcher = SearchEngine.get_search_engine( | ||||||
getattr(settings, "COURSEWARE_CONTENT_INDEX_NAME", "courseware_content") | ||||||
) | ||||||
|
||||||
# Perform the search with the specified query string, size, and field dictionary | ||||||
results = searcher.search( | ||||||
query_string=term, | ||||||
size=limit, | ||||||
field_dictionary={"course": course_id} | ||||||
) | ||||||
|
||||||
# Process the search results to extract relevant fields | ||||||
results = list(map(lambda it: { | ||||||
"id": it["_id"], | ||||||
"display_name": it["data"]["content"]["display_name"], | ||||||
"usage_key": it["_id"], | ||||||
}, results["results"])) | ||||||
|
||||||
return results | ||||||
|
||||||
|
||||||
def auto_suggest_search_api(term, course_id, limit=30): | ||||||
""" | ||||||
Perform an auto-suggest search using the MeiliSearch search engine. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
Args: | ||||||
term (str): The search term. | ||||||
course_id (str): The ID of the course to filter the search results. | ||||||
limit (int, optional): The maximum number of results to return. Defaults to 30. | ||||||
|
||||||
Returns: | ||||||
list: A list of dictionaries containing the search results with 'id', 'display_name' and 'usage_key'. | ||||||
""" | ||||||
# Initialize response dictionary | ||||||
response = {"results": []} | ||||||
|
||||||
# Check which search engine to use based on settings | ||||||
if getattr(settings, "MEILISEARCH_ENABLED", False): | ||||||
# Use MeiliSearch otherwise | ||||||
results = _meilisearch_auto_suggest_search_api(term, course_id, limit) | ||||||
else: | ||||||
# Use Elasticsearch if MEILISEARCH_ENABLED is set to True | ||||||
results = _elasticsearch_auto_suggest_search_api(term, course_id, limit) | ||||||
|
||||||
# Update response with the search results | ||||||
response.update(results=results) | ||||||
|
||||||
return response |
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'm not sure when/if this
settings
file is used, but I don't think we should just setALLOWED_HOSTS = ['*']
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.
It is mostly used during development