feat: FC-0012 - add atlas pull behind a flag

[shadinaif]

(atlas pull) will be performed only if OPENEDX_ATLAS_PULL environment variable is set

Tests:

• in devstack, lms-shell: OPENEDX_ATLAS_PULL=yes make pull_translations is working fine
• in devstack, lms-shell: OPENEDX_ATLAS_PULL=yes make pull_translations is deleting old translations and pulling the latest translations

Note:

I had to run the following command in the container before the test

git config --global --add safe.directory /edx/app/edxapp/edx-platform

References

This pull request is part of the FC-0012 project which is sparked by the Translation Infrastructure update OEP-58.

Check the links above for full information about the overall project.

Internalization is being rearchitected in Open edX Python, XBlock, Micro-frontend, and other projects. There are a number of immediately visible changes:

• Remove source and language translations from the repositories, hence no .json, .po or .mo files will be committed into the repos.
• Add standardized make extract_translations in all repositories
• Push user-facing messages strings into openedx/openedx-translations.
• Integrate root repositories with openedx/openedx-atlas to pull translations on build/deploy time

Breaking Changes

One of the primary goals of the project is to avoid breaking changes. If you noticed any suspicious code, please raise your concern. But before that, please know the strategy we're following to avoid breaking changes

For XBlocks:

• The standard translation path must be conf/locale. Therefore, we are creating a link from conf/locale to translations so Atlas can find the path without disrupting the current way of having translations locally within the XBlocks
• openedx-translations will have a related PR that adds the XBlock to the pipeline. This will not affect the current way of managing/updating translations
• At the end of the project, a clear change log will be added and all translations will be handled by Atlas. Thus, the local translation will be removed from the repo within the version bump
• A notification for the community will be published to ensure that everyone knows why translations directories are removed from all repos

[openedx-webhooks]

Thanks for the pull request, @shadinaif! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[OmarIthawi]

@OmarIthawi , do we need all steps when pulling using Atlas? or we should keep only generate?

@shadinaif I'm not sure, but it's probably a good idea to keep backward compatibility of the workflow.

I've added a suggestion to do that, please let me know what do you think.

[OmarIthawi]

Looks great now. Thanks @shadinaif!

[OmarIthawi]

@shadinaif please add the keyword OEP-58 somewhere in the description so this pull request appears in GitHub search.

[OmarIthawi]

It looks good! @shadinaif I've added few notes regarding the segmented dummy.

Please let me know if you have any questions.

[shadinaif]

Hi @OmarIthawi , please check this out, I believe this is the best way to keep maximum compatibility without TODOs

[OmarIthawi]

@shadinaif I've tested it locally and found this strange error:

Error details

INFO:i18n.execute:Executing in . ...
INFO:i18n.execute:django-admin makemessages -l en -v1 --ignore="*/migrations/*" --ignore="*/envs/*" --ignore="node_modules/*" --ignore="conf/*" --ignore="docs/*" --ignore="*/fonts/*" --ignore="*/img/*" --ignore="*/images/*" --ignore="*/sass/*" --ignore="*/css/*" --ignore="common/test/*" --ignore="test_root/*" --ignore="*/terrain/*" --ignore="*/spec/*" --ignore="*/tests/*" --ignore="*/djangoapps/*/features/*" --ignore="lms/static/js/i18n/*" --ignore="cms/static/js/i18n/*" --ignore="src/acid-xblock/*" --ignore="src/code-block-timer/*" --ignore="src/codejail/*" --ignore="src/django-wiki/*" --ignore="src/done-xblock/*" --ignore="src/parse-rest/*" --ignore="src/geoip2/*" --ignore="src/pystache-custom/*" --ignore="src/rate-xblock/*" --ignore="src/xblock-google-drive/*" -d django
processing locale en
INFO:i18n.execute:Executing in . ...
INFO:i18n.execute:django-admin makemessages -l en -v1 --ignore="*/migrations/*" --ignore="*/envs/*" --ignore="node_modules/*" --ignore="conf/*" --ignore="docs/*" --ignore="*/fonts/*" --ignore="*/img/*" --ignore="*/images/*" --ignore="*/sass/*" --ignore="*/css/*" --ignore="common/test/*" --ignore="test_root/*" --ignore="*/terrain/*" --ignore="*/spec/*" --ignore="*/tests/*" --ignore="*/djangoapps/*/features/*" --ignore="lms/static/js/i18n/*" --ignore="cms/static/js/i18n/*" --ignore="src/acid-xblock/*" --ignore="src/code-block-timer/*" --ignore="src/codejail/*" --ignore="src/django-wiki/*" --ignore="src/done-xblock/*" --ignore="src/parse-rest/*" --ignore="src/geoip2/*" --ignore="src/pystache-custom/*" --ignore="src/rate-xblock/*" --ignore="src/xblock-google-drive/*" -d djangojs -e js,jsx
./cms/static/js/views/course_outline.js:335: warning: unterminated string
./cms/static/js/views/utils/tagging_drawer_utils.js:56: warning: RegExp literal terminated too early
./common/static/common/js/vendor/common.min.js:2: warning: unterminated string
./common/static/js/src/ReactRenderer.jsx:32: warning: unterminated string
./common/static/js/vendor/tinymce/js/tinymce/plugins/codesample/plugin.js:825: warning: unterminated string
./common/static/js/vendor/tinymce/js/tinymce/plugins/codesample/plugin.js:1244: warning: unterminated string
./common/static/js/vendor/tinymce/js/tinymce/tinymce.full.min.js:122: warning: unterminated string
./lms/djangoapps/instructor/static/instructor/ProblemBrowser/data/api/client.js:44: warning: RegExp literal terminated too early
./lms/static/js/demographics_collection/DemographicsCollectionModal.jsx:107: warning: unterminated string
./lms/templates/courseware/progress_graph.js:17: warning: unterminated string
CommandError: errors happened while running msguniq
/edx/app/edxapp/edx-platform/conf/locale/djangojs.pot:2148: context separator <EOT> within string
/edx/app/edxapp/edx-platform/conf/locale/djangojs.pot:2149: context separator <EOT> within string
msguniq: found 2 fatal errors
Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/bin/i18n_tool", line 8, in <module>
    sys.exit(main())
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/i18n/main.py", line 60, in main
    return module.main()
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/i18n/__init__.py", line 50, in __call__
    return self.run(args)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/i18n/extract.py", line 134, in run
    execute(make_djangojs_cmd, working_directory=configuration.root_dir, stderr=stderr)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/i18n/execute.py", line 21, in execute
    sp.check_call(command, cwd=working_directory, stderr=stderr, shell=True)
  File "/usr/lib/python3.8/subprocess.py", line 364, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command 'django-admin makemessages -l en -v1 --ignore="*/migrations/*" --ignore="*/envs/*" --ignore="node_modules/*" --ignore="conf/*" --ignore="docs/*" --ignore="*/fonts/*" --ignore="*/img/*" --ignore="*/images/*" --ignore="*/sass/*" --ignore="*/css/*" --ignore="common/test/*" --ignore="test_root/*" --ignore="*/terrain/*" --ignore="*/spec/*" --ignore="*/tests/*" --ignore="*/djangoapps/*/features/*" --ignore="lms/static/js/i18n/*" --ignore="cms/static/js/i18n/*" --ignore="src/acid-xblock/*" --ignore="src/code-block-timer/*" --ignore="src/codejail/*" --ignore="src/django-wiki/*" --ignore="src/done-xblock/*" --ignore="src/parse-rest/*" --ignore="src/geoip2/*" --ignore="src/pystache-custom/*" --ignore="src/rate-xblock/*" --ignore="src/xblock-google-drive/*" -d djangojs -e js,jsx' returned non-zero exit status 1.
make[1]: *** [Makefile:46: extract_translations] Error 1
make[1]: Leaving directory '/edx/app/edxapp/edx-platform'
make: *** [Makefile:64: pull_translations] Error 2
17:17:38 omar@antilop:edx-platform $ 

This error is intermittent and don't occur all the time. So, I'm proposing a change to this pull request which I've tested:

• fix: use compilemessages zeit-labs/edx-platform#3

Please check it out and let me know if the 79e6c71 commit looks good to you. Of course the other tests PR should be discarded.

[OmarIthawi]

usemanage.py lms compilemessages instead.

[OmarIthawi]

Looks great! Thanks @shadinaif!!

Do you think we need to add the git config --global --add safe.directory /edx/app/edxapp/edx-platform in the Dockerfile to save some steps for the engineers using devstack?

[OmarIthawi]

cc: @feanil this is ready to go.

[feanil]

Generally looks good to me, What's the reason for the git safe.directory setting? Is it because the user that's pulling the translations in docker different from the user that checked out the repo? Also I don't see any dockerfile related changes here, should there be some?

[OmarIthawi]

Generally looks good to me, What's the reason for the git safe.directory setting?
Is it because the user that's pulling the translations in docker different from the user that checked out the repo? Also I don't see any dockerfile related changes here, should there be some?

@shadinaif would you mind checking Feanil's questions?

[OmarIthawi]

@shadinaif please add the git config --global --add safe.directory /edx/app/edxapp/edx-platform to the Dockerfile after you've answered Feanil's questions.

[shadinaif]

Hi @feanil. when using devstack; the command make lms-shell enters the shell as root, not as app. Knowing that app the user owns edx-platform directory. I've added a git command in the Dockerfile. I've added it globally in base stage. Please let me know if there is a reason to avoid that and move it to development stage. The git issue shouldn't happen in production because the admin will enter the shell with correct user anyway

cc:/ @OmarIthawi

[OmarIthawi]

I think this issue is fine within the context of this image. Sometimes the check valid, but it's used in another context that makes it redaudant.

Thanks @shadinaif for the update. I think this pull request is ready to be merged.

cc: @feanil.

[OmarIthawi]

@feanil @brian-smith-tcril @mphilbrick211 would you mind merging this pull request? It's fairly backward compatible and I'd like to rebase the other pull request:

• feat: atlas pull plugins translation | FC-0012 #33922

[openedx-webhooks]

@shadinaif 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.