feat: allow pulling translations by module name rather than repo name #353

OmarIthawi · 2023-05-31T11:47:05Z

In the edx-platform installed XBlocks vary from one installation to another. Therefore, hardcoding the XBlock or plugin list in a Makefile would be cumbersome. Alternatively, the XBlock information is available within the xblock.v1 Python entry points.

When running make pull_translations in the edx-platform, the repository name of the installed XBlock isn't available. On the other hand XBlock's python module name is available.

Rejected alternatives

Hardcode the XBlock names in the edx-platform Makefile and allow maintainers to override the variable name e.g. ATLAS_XBLOCK_REPOS: Similar information is available in the xblock.v1 Python entry points.
Store the GitHub repository name in the XBlock itself e.g. DragAndDropBlock.repo_name == "xblock-drag-and-drop-v2": Similar information is available in the xblock.v1 Python entry points.

TODO

Test locally with feat: experimental atlas and oep-58 i18n service updates FC-0012 edx-platform#31997 on devstack

References

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

Up-to-date project overview and details are available in the Approach Memo and Technical Discovery: Translations Infrastructure Implementation document.

Join the conversation on Open edX Slack #translations-project-fc-0012.

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

For Micro-frontends:

Legacy hardcoded translations are kept into the repo.
Consolidate all i18n imports into src/i18n/index.js
Add atlas integration in make pull_translations but only if OPENEDX_ATLAS_PULL is set
Bump frontend-platform and use intl-imports.js to generate up to date import files
If translations is missing, they're added according to the latest Micro-frontend i18n pattern in par with https://github.com/openedx/frontend-template-application/

openedx-webhooks · 2023-05-31T11:47:10Z

Thanks for the pull request, @OmarIthawi! 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.

brian-smith-tcril

Overall this feels like a good path forward. I left a small comment about a place where the documentation doesn't feel fully clear to me.

python-modules/README.rst

brian-smith-tcril · 2023-05-31T16:21:33Z

python-modules/README.rst

@@ -0,0 +1,11 @@
+Python Modules Links


It's not fully clear to me how this is working. I understand how the links are being created in extract-translation-source-files, and I understand that we need to be able to look things up by module name instead of repo name, but it's not clear to me how this works on the edx-platform side.

Is the plan to update edx-platform to look specifically for a python-modules directory as part of the sparse checkout via atlas?

@brian-smith-tcril yes, this has been tested locally with the edx-platform.

The following is the management command that is responsible about pulling the XBlock translations by module name:

https://github.com/Zeit-Labs/edx-platform/pull/2/files#diff-69c7c1e7f1ec08e9d4f9a4a00fae62392ceaecb644f8ad697888e3eefc4a3bc2R27-R45

We'll share a demo soon to clarify the design we're targeting, but for this PR I think it's ready and it's ready to merge after once you find that the documentation is clear for the purpose of this repository.

I think the biggest thing I've having trouble with is the name of the directory. If we look at the root of openedx-translations now, it's fairly clear what every directory is for:

docs requirements scripts translations

Each directory name clearly implies what the directory is for and what it does. It is not clear from reading python-modules what that directory is for, what it does, and why it lives in the root of the repo.

@brian-smith-tcril yes, sure the name makes more sense now.

I'm wondering about edx-platform-translations? It sounds even more specific, but I'm unaware of the long term plan for the edx-platform repo name though, is there an intention to rename it to openedx-platform?

Anyway, both the directory name and the README.rst file has been updated.

What if they lived in translations/edx-platform-links/? I think having translations and openedx-translations as directories at the top level is confusing.

translations/edx-platform-links/

That should be less confusing. Thanks!

OmarIthawi · 2023-06-05T14:54:03Z

@brian-smith-tcril rename done.

Also tested on our fork:

Workflow manual run: https://github.com/Zeit-Labs/openedx-translations/actions/runs/5178534217/jobs/9330134879
Workflow manual run result commit: Zeit-Labs@5600b8f
Branch tree: https://github.com/Zeit-Labs/openedx-translations/tree/5600b8f06e950821c4c4dbcdea4cd0cd277fff8c/translations/edx-platform-links

Local tree $ ls result:

 audio $ pwd
~/work/openedx/openedx-translations/translations/edx-platform-links/audio
audio $ git status
On branch automated/extract-translation-source-files-20230605T144617
Your branch is up to date with 'origin/automated/extract-translation-source-files-20230605T144617'.

nothing to commit, working tree clean
audio $ ls
conf
audio $ tree
.
└── conf
    └── locale
        └── en
            └── LC_MESSAGES
                └── django.po

4 directories, 1 file

brian-smith-tcril

This looks great! 1 little comment about what appears to be a typo in the readme, other than that this should be good to go!

brian-smith-tcril · 2023-06-05T15:04:09Z

translations/edx-platform-links/README.rst

+Pulling ``edx-platform`` translations with Atlas
+------------------------------------------------
+
+The ``atlass`` command has been integrated in the ``edx-platform`` repository therefore there's no need to run ``atlas pull`` manually. Regardless, this section shows the manual command arguments because it's needed for some advanced usage of the edX Platform such as installing custom XBlocks that's not part of the openedx github organizaiton.


is atlass here (with the double s) a typo?

yup, and fixed

…po name This pull request is part of the [FC-0012 project](https://openedx.atlassian.net/l/cp/XGS0iCcQ) which is sparked by the [Translation Infrastructure update OEP-58](https://open-edx-proposals.readthedocs.io/en/latest/architectural-decisions/oep-0058-arch-translations-management.html#specification).

openedx-webhooks · 2023-06-05T16:11:16Z

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

openedx#353 it's no longer needed because we can pull with glob patterns now see https://github.com/openedx/openedx-atlas/blob/main/docs/decisions/0001-support-glob-pattern.rst Refs: FC-0012 OEP-58

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label May 31, 2023

OmarIthawi marked this pull request as ready for review May 31, 2023 11:47

OmarIthawi force-pushed the module-names branch 2 times, most recently from 24ffa49 to f4292a8 Compare May 31, 2023 16:01

brian-smith-tcril reviewed May 31, 2023

View reviewed changes

OmarIthawi force-pushed the module-names branch 2 times, most recently from 8491d3d to f597821 Compare June 2, 2023 04:15

OmarIthawi requested a review from brian-smith-tcril June 2, 2023 04:15

OmarIthawi force-pushed the module-names branch 4 times, most recently from 8778c3d to e13d740 Compare June 5, 2023 14:45

brian-smith-tcril reviewed Jun 5, 2023

View reviewed changes

OmarIthawi force-pushed the module-names branch from e13d740 to bfc9adf Compare June 5, 2023 16:05

OmarIthawi force-pushed the module-names branch from bfc9adf to 572111a Compare June 5, 2023 16:06

OmarIthawi requested a review from brian-smith-tcril June 5, 2023 16:08

brian-smith-tcril approved these changes Jun 5, 2023

View reviewed changes

brian-smith-tcril merged commit 126093b into openedx:main Jun 5, 2023

OmarIthawi mentioned this pull request Jun 6, 2023

feat: Add xblock-adventure to the translation pipeline #251

Closed

1 task

shadinaif mentioned this pull request Sep 20, 2023

revert: allow pulling translations by module name rather than repo name #1205

Merged

OmarIthawi mentioned this pull request Nov 18, 2023

feat: Remove the edx-platform-links from translations repo #1067

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow pulling translations by module name rather than repo name #353

feat: allow pulling translations by module name rather than repo name #353

OmarIthawi commented May 31, 2023 •

edited

Loading

openedx-webhooks commented May 31, 2023 •

edited

Loading

brian-smith-tcril left a comment

brian-smith-tcril May 31, 2023

OmarIthawi Jun 1, 2023

brian-smith-tcril Jun 1, 2023

OmarIthawi Jun 2, 2023 •

edited

Loading

brian-smith-tcril Jun 2, 2023

OmarIthawi Jun 5, 2023

OmarIthawi commented Jun 5, 2023 •

edited

Loading

brian-smith-tcril left a comment

brian-smith-tcril Jun 5, 2023

OmarIthawi Jun 5, 2023

openedx-webhooks commented Jun 5, 2023

feat: allow pulling translations by module name rather than repo name #353

feat: allow pulling translations by module name rather than repo name #353

Conversation

OmarIthawi commented May 31, 2023 • edited Loading

Rejected alternatives

TODO

References

Breaking Changes

openedx-webhooks commented May 31, 2023 • edited Loading

brian-smith-tcril left a comment

Choose a reason for hiding this comment

brian-smith-tcril May 31, 2023

Choose a reason for hiding this comment

OmarIthawi Jun 1, 2023

Choose a reason for hiding this comment

brian-smith-tcril Jun 1, 2023

Choose a reason for hiding this comment

OmarIthawi Jun 2, 2023 • edited Loading

Choose a reason for hiding this comment

brian-smith-tcril Jun 2, 2023

Choose a reason for hiding this comment

OmarIthawi Jun 5, 2023

Choose a reason for hiding this comment

OmarIthawi commented Jun 5, 2023 • edited Loading

brian-smith-tcril left a comment

Choose a reason for hiding this comment

brian-smith-tcril Jun 5, 2023

Choose a reason for hiding this comment

OmarIthawi Jun 5, 2023

Choose a reason for hiding this comment

openedx-webhooks commented Jun 5, 2023

OmarIthawi commented May 31, 2023 •

edited

Loading

openedx-webhooks commented May 31, 2023 •

edited

Loading

OmarIthawi Jun 2, 2023 •

edited

Loading

OmarIthawi commented Jun 5, 2023 •

edited

Loading