feat: add ElasticSearch support in tutor-discovery #89
Conversation
| @@ -0,0 +1 @@ | |||
| - [Feature] Add `ElasticSearch` support in tutor-discovery as tutor core is shifted to `MeiliSearch`. (by @Faraz32123) | |||
There was a problem hiding this comment.
We can link core PR here and maybe tag this as a breaking change?
There was a problem hiding this comment.
yup, we can do that.
There was a problem hiding this comment.
No need to wrap Elasticsearch and Meilisearch in backticks (and there is no capital S)
| {% if DISCOVERY_RUN_ELASTICSEARCH and is_docker_rootless() %} | ||
| elasticsearch: | ||
| ulimits: | ||
| memlock: | ||
| # Fixes error setting rlimits for ready process in rootless docker | ||
| soft: 0 # zero means "unset" in the memlock context | ||
| hard: 0 | ||
| {% endif %} |
There was a problem hiding this comment.
these settings are necessary for elasticsearch i think, when we are running docker in rootless mode to avoid memory issues for elasticsearch.
| DISCOVERY_DOCKER_IMAGE_ELASTICSEARCH = "{{ DISCOVERY_DOCKER_IMAGE_ELASTICSEARCH }}" | ||
| DISCOVERY_ELASTICSEARCH_HOST = "{{ DISCOVERY_ELASTICSEARCH_HOST }}" | ||
| DISCOVERY_ELASTICSEARCH_PORT = "{{ DISCOVERY_ELASTICSEARCH_PORT }}" | ||
| DISCOVERY_ELASTICSEARCH_SCHEME = "{{ DISCOVERY_ELASTICSEARCH_SCHEME }}" | ||
| DISCOVERY_ELASTICSEARCH_HEAP_SIZE = "{{ DISCOVERY_ELASTICSEARCH_HEAP_SIZE }}" |
There was a problem hiding this comment.
As we are running elasticsearch container for/from discovery plugin now, these settings will be need in various patches to run elasticsearch same as it was running through tutor.
There was a problem hiding this comment.
But if the patches were expecting the config with DISCOVERY_ prefix, won't those patches need to an update as well?
There was a problem hiding this comment.
yeah, these settings are already being used in all patches in this PR with DISCOVERY_ prefix.
| ELASTIC_SEARCH_CONFIG = [{ | ||
| "host": "{{ DISCOVERY_ELASTICSEARCH_HOST }}", | ||
| "port": "{{ DISCOVERY_ELASTICSEARCH_PORT }}", | ||
| }] | ||
| {% if "{{ DISCOVERY_ELASTICSEARCH_SCHEME }}" == "https" %}": | ||
| ELASTIC_SEARCH_CONFIG[0]["use_ssl"] = True | ||
| {% endif %} |
There was a problem hiding this comment.
I think, these settings can be removed as course-discovery doesn't use this and elasticsearch doesn't have such default settings variable, plus it was previously used in edx-devstack.
Faraz32123
force-pushed
the
feat/enable_elastic_search_for_discovery
branch
3 times, most recently
from
e9a9787
to
e6fc6e9
Compare
| @@ -0,0 +1 @@ | |||
| {% if DISCOVERY_RUN_ELASTICSEARCH %}setowner 1000 /mounts/elasticsearch{% endif %} | |||
There was a problem hiding this comment.
I am seeing stat: can't stat '/mounts/elasticsearch': No such file or directory error in permissions container. Can you verify this, please?
| @@ -0,0 +1 @@ | |||
| - [Feature] Add `ElasticSearch` support in tutor-discovery as tutor core is shifted to `MeiliSearch`. (by @Faraz32123) | |||
There was a problem hiding this comment.
No need to wrap Elasticsearch and Meilisearch in backticks (and there is no capital S)
Faraz32123
force-pushed
the
feat/enable_elastic_search_for_discovery
branch
from
e6fc6e9
to
eddda6f
Compare
- As Tutor and Open edX have shifted to Meilisearch, and course-discovery still depends on Elasticsearch, running the Elasticsearch container with tutor-discovery will facilitate smoother operation for the course-discovery service. - It's related PR from tutor: overhangio/tutor#1141.
Faraz32123
force-pushed
the
feat/enable_elastic_search_for_discovery
branch
from
eddda6f
to
dd0fd6e
Compare
yeah, permission container had the error for /mounts/elasticsearch. I have resolved that now. Thanks for testing it out at your end :) |
- As Tutor and Open edX have shifted to Meilisearch, and course-discovery still depends on Elasticsearch, running the Elasticsearch container with tutor-discovery will facilitate smoother operation for the course-discovery service. - It's related PR from tutor: overhangio/tutor#1141. Co-authored-by: Muhammad Faraz Maqsood <[email protected]>
ElasticSearchsupport in tutor-discovery as tutor core is shifted toMeiliSearch.