Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

feat: Switch to Atlas translations in edxapp (attempt 3) #7119

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

timmc-edx
Copy link
Contributor

The previous attempt (#7117) was failing with an error in the call to paver i18n_compilejs after the Atlas-specific part of pull_translations, in the part where it runs npm ci. I don't have access to the npm log, but I bet it's just that our npm setup wasn't ready yet at this point in the playbook.

I think we just need to move this call to after the npm setup.

Part of OEP-58 and edx/edx-arch-experiments#548

Configuration Pull Request

Make sure that the following steps are done before merging:

  • Have a Site Reliability Engineer review the PR if you don't own all of the services impacted.
  • If you are adding any new default values that need to be overridden when this change goes live, update internal repos and add an entry to the top of the CHANGELOG.
  • Performed the appropriate testing.
  • Think about how this change will affect Open edX operators and update the wiki page for the next Open edX release if needed

The previous attempt (#7117) was failing with an error in the call to
`paver i18n_compilejs` after the Atlas-specific part of pull_translations,
in the part where it runs `npm ci`. I don't have access to the npm log, but
I bet it's just that our npm setup wasn't ready yet at this point in the
playbook.

I think we just need to move this call to after the npm setup.

Part of OEP-58 and edx/edx-arch-experiments#548
@timmc-edx timmc-edx merged commit 86eec47 into master Feb 23, 2024
4 checks passed
@timmc-edx timmc-edx deleted the timmc/atlas-after-npm branch February 23, 2024 19:13
timmc-edx added a commit that referenced this pull request Feb 23, 2024
timmc-edx added a commit that referenced this pull request Feb 23, 2024
timmc-edx added a commit that referenced this pull request Mar 1, 2024
This is to support OEP-58:
https://docs.openedx.org/projects/openedx-proposals/en/latest/architectural-decisions/oep-0058-arch-translations-management.html

Changes since the #7119 attempt:

- Move to just after main requirements are installed (just for faster feedback during testing, really)
- Use already-installed `openedx-atlas`
- Use variant-agnostic `EDX_PLATFORM_SETTINGS` rather than `DJANGO_SETTINGS_MODULE`
- Use Ansible `environment` field for env vars
- Drop `set -eu -o pipefail` since the script is now simple

Most importantly, openedx/edx-platform#34306 has since merged, so we don't have to worry about the NPM conflicts we were getting previously.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants