Skip to content
This repository was archived by the owner on Aug 3, 2026. It is now read-only.

[BB-6112] Prepare opencraft-release/maple.3 branches for configuration - #193

Merged
0x29a merged 7 commits into
opencraft-release/maple.3from
0x29a/bb6112/prepare-maple-configuration-shared-branch
May 30, 2022
Merged

[BB-6112] Prepare opencraft-release/maple.3 branches for configuration#193
0x29a merged 7 commits into
opencraft-release/maple.3from
0x29a/bb6112/prepare-maple-configuration-shared-branch

Conversation

@0x29a

@0x29a 0x29a commented May 4, 2022

Copy link
Copy Markdown

Description

This PR prepares the shared opencraft-release/maple.3 branch for our fork of https://github.com/openedx/configuration.

Code-drift commits (from the oldest to the newest):

Commit Upstream PR Included in open-release/maple.3 Should be cherry-picked? Notes
1 openedx-unsupported/configuration@694e6ce link openedx-unsupported/configuration@f731c07
2 openedx-unsupported/configuration@a07798c
3 openedx-unsupported/configuration@0573de1 We might not need this: 1, 2.
4 openedx-unsupported/configuration@1116e75 link openedx-unsupported/configuration@adde3a2 Details.
5 openedx-unsupported/configuration@e0219d1 link openedx-unsupported/configuration@b0f7e53
6 openedx-unsupported/configuration@b2e8606 link openedx-unsupported/configuration@adde3a2 No need to cherry-pick. See the notes for the commit №4.
7 openedx-unsupported/configuration@b5eeb9b No need to cherry-pick. See the commit №8.
8 openedx-unsupported/configuration@06fb8f4 Requires cherry-picking and the version update.
9 openedx-unsupported/configuration@0252892 Merge commit, no need to cherry-pick.
10 openedx-unsupported/configuration@d078cb2 link openedx-unsupported/configuration@2587f23
11 openedx-unsupported/configuration@c50b638 link openedx-unsupported/configuration@2587f23
12 openedx-unsupported/configuration@30e9994 link openedx-unsupported/configuration@5bd1156
13 openedx-unsupported/configuration@dd6bb7a link We've missed backporting this to Maple.
14 openedx-unsupported/configuration@b551572 link openedx-unsupported/configuration@5bd1156 Merge commit, no need to cherry-pick.
15 openedx-unsupported/configuration@98dab5f link Merge commit, no need to cherry-pick.
16 openedx-unsupported/configuration@9933e7c link Created after the open-release/maple.3 tagging.
17 openedx-unsupported/configuration@3d6a26f link Merge-commit, no need to cherry-pick.
18 openedx-unsupported/configuration@4bb4424 link Created after the open-release/maple.3 tagging.
19 openedx-unsupported/configuration@1dbf3ba link Merge-commit, no need to cherry-pick.
20 openedx-unsupported/configuration@8107593 link Created after the open-release/maple.3 tagging.
21 openedx-unsupported/configuration@7e34794 link This hasn't been backported to Maple. See BB-6123.

Total number of commits to cherry-pick: 7.

Note: I had to cherry-pick the additional commit (2d7caab), which is the part of the commit 21. Details.

Testing instructions

Other information

Suggested sandbox configuration:

# needed for the testing of https://github.com/open-craft/configuration/pull/154
newrelic_query_key: ""
newrelic_query_api_url: ""
uptime_report_s3_bucket: ""

# needed for the testing of https://github.com/open-craft/configuration/pull/159
EDXAPP_REINDEX_ALL_COURSES: true
EDXAPP_FEATURES_EXTRA:
  ENABLE_COURSEWARE_INDEX: true
  ENABLE_COURSEWARE_SEARCH: true
  ENABLE_DASHBOARD_SEARCH: true
  ENABLE_LIBRARY_INDEX: true

# needed for the testing of https://github.com/openedx/configuration/pull/6679
# https://github.com/openedx/configuration/pull/6708
# https://github.com/openedx/configuration/pull/6677
# and https://github.com/openedx/edx-platform/pull/29851
EDXAPP_DEFAULT_SITE_THEME: simple-theme
SIMPLETHEME_ENABLE_DEPLOY: true
EDXAPP_COMPILE_JSI18N: true
EDXAPP_COMPREHENSIVE_THEME_LOCALE_PATHS:
  - "{{ EDXAPP_COMPREHENSIVE_THEME_DIRS[0] }}/conf/locale"
EDXAPP_PREPEND_LOCALE_PATHS:
  - /edx/src/
  - "{{ EDXAPP_COMPREHENSIVE_THEME_DIRS[0] }}/simple-theme/i18n/conf/locale"
SIMPLETHEME_I18N_DJANGO:
  - lang: ar
    domain: django
    headers: |
      "Plural-Forms: nplurals=6; plural=n==0 ? 0 : n==1 ? 1 : n==2 ? 2 : n%100>=3 && n%100<=10 ? 3 : n%100>=11 && n%100<=99 ? 4 : 5;\n"
    messages: |
      msgid "View Course"
      msgstr "XXX استعراض المحتوى"
  - lang: ar
    domain: djangojs
    headers: |
      "Plural-Forms: nplurals=6; plural=n==0 ? 0 : n==1 ? 1 : n==2 ? 2 : n%100>=3 && n%100<=10 ? 3 : n%100>=11 && n%100<=99 ? 4 : 5;\n"
    messages: |
      msgid "Submit"
      msgstr "تقديم YYY"
      msgid "Add a Post"
      msgstr "إضافة منشور ZZZ"

@0x29a

0x29a commented May 28, 2022

Copy link
Copy Markdown
Author

Testing notes:

@0x29a
0x29a force-pushed the 0x29a/bb6112/prepare-maple-configuration-shared-branch branch from 2d7caab to 8fdc460 Compare May 28, 2022 15:57
@0x29a

0x29a commented May 28, 2022

Copy link
Copy Markdown
Author

@mtyaka, I'm trying to test openedx-unsupported#6679 and openedx-unsupported#6708 using this sandbox, but, despite seeing the expected django.po and djangojs.po (along the compiled .mo files) in /edx/var/edxapp/themes/simple-theme/i18n/conf/locale/ar/LC_MESSAGES, I don't see YYY and ZZZ with ar locale enabled. Do you know what I can be missing?

@mtyaka

mtyaka commented May 30, 2022

Copy link
Copy Markdown
Member

@0x29a I think the reason you're not seeing the custom translations is that you need to use EDXAPP_PREPEND_LOCALE_PATHS instead of EDXAPP_COMPREHENSIVE_THEME_LOCALE_PATHS, otherwise the default translations take precedence.
For reference, custom translations are set up on this instance.

@0x29a

0x29a commented May 30, 2022

Copy link
Copy Markdown
Author

@mtyaka, ah, looks like the sandbox config here has a mistake (it doesn't set EDXAPP_PREPEND_LOCALE_PATHS)?

Anyway, I moved locale paths from COMPREHENSIVE_THEME_LOCALE_PATHS to PREPEND_LOCALE_PATHS, ran compilejsi18n and /edx/bin/edxapp-update-assets on the app server, restarted services, and finally was able to see إضافة منشور ZZZ and تقديم YYY.

Thank you very much!

@Agrendalath

Agrendalath commented May 30, 2022

Copy link
Copy Markdown
Member

@0x29a, this looks great. Thank you for providing all references to upstream PRs, backports, and commits, and for testing these changes!

I have only these small comments, but they are not a blocker.

Can we rebase opencraft-release/maple.3 on open-release/maple.master, and then rebase this branch? Let's include this commit and drop 8fdc460 this way.

white_check_mark #159: I had to add features manually to the /edx/etc/lms.yml, as I made a mistake in a suggested configuration, and executed python manage.py cms reindex_course --all. It worked fine.

Nit: see my comment (quoted below for convenience) on the internal ticket. This is not the correct way to test this, but my explanation is enough for me to give a +1 for this cherry-pick. We might need small adjustments for this solution, but it's not in the scope of the current PR.

In short - we're deploying ElasticSearch on each Ocim sandbox, so the search data is stored only locally. Therefore, we need to rebuild it after each deployment. Moving it to the forum playbook to make it run after the local ElasticSearch playbook technically works, but we can achieve a more elegant (and upstreamable) solution by moving the ES role above edxapp.

As long as we have only one active AppServer at a time, this is going to work. If we want to achieve data persistence, we should either deploy a dedicated ES or wait until SE-5362 is completed.

I just don't understand why we've changed --setup to --all. The main difference is disabling the confirmation for a non-interactive session, which sounds like something we would like to keep.

@Agrendalath Agrendalath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

  • I tested this: verified testing instructions from upstream and testing steps from this
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

Sandeep Choudhary and others added 7 commits May 30, 2022 17:35
#164

(cherry picked from commit 590f352)
(cherry picked from commit a07798c)
… it into forum role (#159)

#159

(cherry picked from commit 066fbf5)
(cherry picked from commit 0573de1)
Previously, we pinned to an opencraft release as a quick fix,
but that quick fix is now merged to lilac upstream.
(The fix being https://github.com/edx/tubular/pull/531 )
It didn't make it to lilac.2, so we need to pin to lilac.master.

(cherry picked from commit 06fb8f4)
…5299_prepend_local_paths

feat: add PREPEND_LOCALE_PATHS configuration setting
(cherry picked from commit e397261)
…-5140-simple-theme-i18n

feat: add SIMPLETHEME_I18N_DJANGO variable
(cherry picked from commit ae8100e)
…-5140-collect-js-translations

feat: optionally collect JS translations on deploy
(cherry picked from commit 371e360)
…it_clone

fix: Use repo owner user for git operations
(cherry picked from commit 8f5cea0)
@0x29a
0x29a force-pushed the 0x29a/bb6112/prepare-maple-configuration-shared-branch branch from 8fdc460 to 0248d99 Compare May 30, 2022 15:36
@0x29a
0x29a merged commit 537f65f into opencraft-release/maple.3 May 30, 2022
@0x29a
0x29a deleted the 0x29a/bb6112/prepare-maple-configuration-shared-branch branch May 30, 2022 15:38
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.

5 participants