feat: switching to local translations for IDAs #193
Conversation
- atlas using IDAs (credentials and discovery) switched to using local translations - fixes a mistaken removal of creds transltions from #153 - in credentials, switches a dedicated run of to a more complete translation-building tested the docker build locally FIXES: AU-2730
There was a problem hiding this comment.
Pull request overview
This pull request switches the credentials and course-discovery IDAs to use local translations by configuring the OPENEDX_TRANSLATIONS_REPO build argument and setting the ATLAS_OPTIONS environment variable. The changes fix a previous removal of credential translations and improve the translation building process.
Changes:
- Added OPENEDX_TRANSLATIONS_REPO build argument and ATLAS_OPTIONS environment variable to both credentials and course-discovery Dockerfiles
- Modified credentials Dockerfile to use
make staticinstead of just running webpack directly, which includes translation compilation - Added
make pull_translationsto the credentials base stage to fetch translations during build
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| dockerfiles/credentials.Dockerfile | Adds translation repository configuration, pulls translations in base stage, and switches to make static for more complete asset building |
| dockerfiles/course-discovery.Dockerfile | Adds translation repository configuration to prod and dev stages with ATLAS_OPTIONS environment variable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RUN curl -L -o credentials/settings/devstack.py https://raw.githubusercontent.com/edx/devstack/${CREDENTIALS_SERVICE_VERSION}/py_configuration_files/credentials.py | ||
|
|
||
| ENV DJANGO_SETTINGS_MODULE=credentials.settings.devstack | ||
| ENV ATLAS_OPTIONS="--repository=$OPENEDX_TRANSLATIONS_REPO" |
There was a problem hiding this comment.
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.
| # SecretsUsedInArgOrEnv check gets false positives on the name CREDENTIALS | ||
| FROM ubuntu:jammy AS base | ||
|
|
||
| ARG OPENEDX_TRANSLATIONS_REPO |
There was a problem hiding this comment.
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.
| ARG OPENEDX_TRANSLATIONS_REPO | |
| ARG OPENEDX_TRANSLATIONS_REPO=edx/openedx-translations |
| ENV TZ=UTC | ||
| ENV TERM=xterm-256color | ||
| ENV DEBIAN_FRONTEND=noninteractive | ||
| ENV ATLAS_OPTIONS="--repository=$OPENEDX_TRANSLATIONS_REPO" |
There was a problem hiding this comment.
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.
| FROM app AS prod | ||
|
|
||
| ENV DJANGO_SETTINGS_MODULE "course_discovery.settings.production" | ||
| ARG OPENEDX_TRANSLATIONS_REPO |
There was a problem hiding this comment.
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.
| ARG OPENEDX_TRANSLATIONS_REPO | ||
|
|
||
| ENV DJANGO_SETTINGS_MODULE="course_discovery.settings.production" | ||
| ENV ATLAS_OPTIONS="--repository=$OPENEDX_TRANSLATIONS_REPO" |
There was a problem hiding this comment.
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 86 to fail or use incorrect settings. The ARG should have a default value to prevent this issue.
|
|
||
| FROM app AS dev | ||
|
|
||
| ARG OPENEDX_TRANSLATIONS_REPO |
There was a problem hiding this comment.
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.
|
|
||
| ENV DJANGO_SETTINGS_MODULE "course_discovery.settings.devstack" | ||
| ENV DJANGO_SETTINGS_MODULE="course_discovery.settings.devstack" | ||
| ENV ATLAS_OPTIONS="--repository=$OPENEDX_TRANSLATIONS_REPO" |
There was a problem hiding this comment.
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 102 to fail or use incorrect settings. The ARG should have a default value to prevent this issue.
deborahgu
deleted the
dkaplan1/AU-2730_enable-forked-translations-in-localized-idas
branch
tested the docker build locally to make sure the translations build correctly.
FIXES: AU-2730