-
Notifications
You must be signed in to change notification settings - Fork 4
feat: switching to local translations for IDAs #193
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -76,7 +76,10 @@ EXPOSE 8381 | |
|
|
||
| FROM app AS prod | ||
|
|
||
| ENV DJANGO_SETTINGS_MODULE "course_discovery.settings.production" | ||
| ARG OPENEDX_TRANSLATIONS_REPO | ||
|
|
||
| ENV DJANGO_SETTINGS_MODULE="course_discovery.settings.production" | ||
| ENV ATLAS_OPTIONS="--repository=$OPENEDX_TRANSLATIONS_REPO" | ||
|
||
|
|
||
| RUN pip install -r ${DISCOVERY_CODE_DIR}/requirements/production.txt | ||
|
|
||
|
|
@@ -86,9 +89,12 @@ CMD gunicorn --bind=0.0.0.0:8381 --workers 2 --max-requests=1000 -c course_disco | |
|
|
||
| FROM app AS dev | ||
|
|
||
| ARG OPENEDX_TRANSLATIONS_REPO | ||
|
||
|
|
||
| RUN curl -L -o ${DISCOVERY_CODE_DIR}/course_discovery/settings/devstack.py https://raw.githubusercontent.com/edx/devstack/master/py_configuration_files/course_discovery.py | ||
|
|
||
| ENV DJANGO_SETTINGS_MODULE "course_discovery.settings.devstack" | ||
| ENV DJANGO_SETTINGS_MODULE="course_discovery.settings.devstack" | ||
| ENV ATLAS_OPTIONS="--repository=$OPENEDX_TRANSLATIONS_REPO" | ||
|
||
|
|
||
| RUN pip install -r ${DISCOVERY_CODE_DIR}/requirements/django.txt | ||
| RUN pip install -r ${DISCOVERY_CODE_DIR}/requirements/local.txt | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,12 +3,14 @@ | |||||
| # SecretsUsedInArgOrEnv check gets false positives on the name CREDENTIALS | ||||||
| FROM ubuntu:jammy AS base | ||||||
|
|
||||||
| ARG OPENEDX_TRANSLATIONS_REPO | ||||||
|
||||||
| ARG OPENEDX_TRANSLATIONS_REPO | |
| ARG OPENEDX_TRANSLATIONS_REPO=edx/openedx-translations |
Copilot
AI
Jan 26, 2026
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.
The ATLAS_OPTIONS environment variable is being set with a reference to $OPENEDX_TRANSLATIONS_REPO, but this build argument may be undefined if not passed during build time. This could result in ATLAS_OPTIONS="--repository=" (empty repository value), which would cause the pull_translations command at line 114 to fail or use incorrect settings. The ENV directive should be moved after the repository code is cloned, or the ARG should have a default value.
Copilot
AI
Jan 26, 2026
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.
The ATLAS_OPTIONS environment variable is being set with a reference to $OPENEDX_TRANSLATIONS_REPO, but this build argument may be undefined if not passed during build time. This could result in ATLAS_OPTIONS="--repository=" (empty repository value), which would cause the pull_translations command at line 143 to fail or use incorrect settings. The ARG should have a default value to prevent this issue.
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.
The OPENEDX_TRANSLATIONS_REPO build argument lacks a default value. This will cause build failures if the argument is not explicitly provided. Looking at edx-platform.Dockerfile line 25, a default value like "edx/openedx-translations" is used. Consider adding a default value to maintain consistency with other services and ensure builds work without requiring this argument to be explicitly passed.