-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: add OEP-0058 - Translations Management #367
Changes from 21 commits
6a7f323
d4f6b7a
2f97f83
bd779ed
6f575d0
8826ec0
89f2284
58c39ac
0ae30b1
a72f901
228e3cd
77dbb0e
01cbfba
87ab993
3638508
588e3a6
973cf3e
116ded7
2836cde
0119570
20b9e6c
8f63bbe
224a5df
de68683
cfac57d
8c1a57a
0061121
b9e5abc
59bf579
b501020
c0d2755
b0a0a06
4587332
59a257d
8fcb11e
e00b253
a550fc8
f726086
6a9df8f
9474f40
7c67da0
e9a2c70
17190ad
daf503c
c839424
fc07406
3b40a06
1d27a59
620e6b3
7a06a16
6e2de68
e86e67d
4fbff9e
eeaf460
df76645
3e14956
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,210 @@ | ||
OEP-58: Translations Management | ||
############################### | ||
|
||
.. list-table:: | ||
:widths: 25 75 | ||
|
||
* - OEP | ||
- :doc:`OEP-58 <oep-0058-arch-translations-management>` | ||
* - Title | ||
- Translations Management | ||
* - Last Modified | ||
- 2022-08-10 | ||
* - Authors | ||
- | ||
* Carlos Muniz <[email protected]> | ||
* Feanil Patel <[email protected]> | ||
* Sarina Canelake <[email protected]> | ||
* - Arbiter | ||
- Ned Batchelder <[email protected]> | ||
* - Status | ||
- Under Review | ||
* - Type | ||
- Architecture Decision | ||
* - Created | ||
- 2022-08-08 | ||
* - Review Period | ||
- TBD | ||
.. * - Resolution | ||
.. - | ||
|
||
.. contents:: | ||
:local: | ||
:depth: 1 | ||
|
||
Context | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most OEPs are structured in some variation of:
I find that this template makes it really easy to digest OEPs, both as a first time reader and as a returning reference-seeker. I find the same thing to be true with PEPs. I think the current draft of this OEP could be mapped into the normal OEP template without any major edits:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for this suggestion. I will reorganize the OEP this way at the end of the review period so comments on specific sections are referenced correctly by GitHub. |
||
******* | ||
|
||
There are problems with the current method of managing translations via the | ||
edx-transifex-bot. The edx-transifex-bot uses a soon to be deprecated (Nov 30, 2022) | ||
version of the Transifex API, and requires admin rights on Transifex in order to | ||
function. In addition, it is difficult to track why some PRs are merged by the bot, and | ||
some are not, and where the bot is creating and merging PRs. Most recently, it was | ||
discovered that the translations were not uploading properly but it has been impossible | ||
to debug exactly why. In the week before the Nutmeg release, this was a significant pain | ||
point. | ||
|
||
Decision | ||
******** | ||
|
||
To alleviate these issues, we will switch from using the edx-transifex-bot to the GitHub | ||
Transifex App, a stable app provided by Transifex. Benefits of this change include being | ||
easier to maintain and solving a lot of the pain points. As part of this proposal, we | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
suggest moving translations into their own repository, to make using the GitHub | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Transifex app more streamlined and straightforward, and in order to make organizing and | ||
using the up to date translations simpler. | ||
|
||
Current State | ||
************* | ||
|
||
* Edx-transifex-bot is a potential security issue: The edx-transifex-bot requires admin | ||
rights on Transifex in order to function. Admin rights give access to private/sensitive | ||
information as well as the ability to permanently delete translation and configuration | ||
files. At some point, the login to the edx-transifex-bot user was lost, and without | ||
access to the scripts that the bot uses to function, this edx-transifex-bot is a | ||
security issue we cannot control or debug. | ||
* Edx-transifex-bot is a black box: Login/scripts for edx-transifex-bot cannot be found | ||
on GitHub or on wiki/secrets pages, and it is difficult to observe what work it is | ||
supposed to do, or whether it is doing it correctly. | ||
* The translations for the Open edX Maple Release were never uploaded to Transifex, | ||
because the automation handled by the edx-transifex-bot never uploaded it. | ||
* The translations for the Open edX Nutmeg Release were nearly not uploaded because the | ||
edx-transifex-bot did not automatically upload it. In addition, the script for | ||
uploading the translations did not work, and the script had to be tinkered for the new | ||
Transifex API in order to upload it in time for release. The files uploaded were still | ||
not in the preferred format because the new version of the Transifex API does not have | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
all the same features as the previous API version. | ||
|
||
* Running the script to upload translations requires Transifex admin rights, and there | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
are few people with Admin rights to Transifex and knowledge of the Transifex API. We | ||
should use the solution provided by Transifex to solve this type of problem. | ||
|
||
Rationale | ||
********* | ||
|
||
* This is an upgrade of a system we use regularly, but do not want to have to maintain | ||
regularly. | ||
* Upgrading from a bot (machine user) to an app/workflow is recommended by GitHub and | ||
makes the translation process more open source. | ||
* The GitHub Transifex Integration App is developed and supported by Transifex | ||
* The Github Transifex App is very simple to configure and with many options. We can set | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Transifex Projects to automatically upload/download translation files from a repository | ||
once the translations are reviewed and accepted. | ||
* Transifex only allows a one-to-one relationship between repositories and Transifex | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Projects. Organizing all of the translation files into one repository and one Transifex | ||
Project has a lower cost: we pay by the project so we end up paying less, and by | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
putting all translation files in the same place, it is easier to track translation | ||
progress, and debug translation issues. | ||
* A repository that only contains text/binary files, and uses branches to separate | ||
translations related to Open edX releases can make all interactions with translations | ||
very quick and simple due to the ability to clone the branch of a specific release, | ||
with translations organized by repository name. | ||
* By using an app that is maintained by Transifex the organization, we reduce the | ||
maintenance burden and are more future proof of changes they might make since they | ||
maintain both the API and the github App. | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Proposed Implementation | ||
*********************** | ||
|
||
Move translation files to a new repo | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
==================================== | ||
|
||
Translation files (of types .mo and .po) currently exist amongst the code/documentation | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
they translate. We will move these Translation files from being amongst the | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
code/documentation to their own repo. For easier reintegration, Translation files will be | ||
kept in the same directory structure as the code/documentation they translate. | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Add GitHub Transifex App to openedx organization | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
================================================ | ||
|
||
Transifex's GitHub app will need to be added to the openedx GitHub organization in order | ||
to grant the app permissions to push/pull the Translation files. Currently, we manage the | ||
push/pull permissions for the edx-transifex-bot through a number of groups. The Transifex | ||
GitHub app is granted permission on a repository basis, and by moving all the | ||
Translations to a single repository we reduce the number of user groups we will have to | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
manage. | ||
|
||
Connect i18n repository to Transifex via app | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
============================================ | ||
|
||
The Transifex webapp accepts configuration files for each Transifex project. By | ||
connecting the single repository containing all Translation files, we only need to make a | ||
single configuration file that allows the Transifex GitHub app to manage the Translation | ||
files. Based on the Translation Working Group's instruction, we can set parameters that | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
automatically push and pull Translation files. | ||
|
||
Copy Transifex Memory | ||
===================== | ||
|
||
As a last step we can save all the progress the Open edX translators have accomplished by | ||
copying the Transifex Memory, the auto-translation feature that allows for Projects with | ||
similar strings to be automatically translated, from the old projects to this new one. By | ||
moving all the Translation Files to the same repository we can increase the reach of the | ||
Transifex Memory feature to help translate similar strings across the entire | ||
code/documentation base. | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Impacts | ||
******* | ||
|
||
Impact on Translators | ||
===================== | ||
|
||
As we approach the end of the translation upgrade process, we will need to tactically | ||
move from multiple transifex projects to a single project. This will require | ||
coordination with our translators to ensure that moving forward they are providing | ||
translations in the right place. | ||
|
||
Impact on Site Operators | ||
======================== | ||
|
||
Currently the translation files for any given service or library is stored at the same | ||
place as the code, this has generally simplified the deployment story in the past. With | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this change, the translations files will move to their own repository. As we deprecate | ||
the old translations files, the relevant deployment tooling will need to be updated to | ||
pull down the translations from the new repository as a part of the deployment process. | ||
This will impact both the old Ansible based tooling as well as any new docker based | ||
tooling. | ||
|
||
Impact on Developers | ||
==================== | ||
|
||
While it won’t directly impact the day-to-day workflow of developers, due to the same | ||
reasons that we impact site operators(new translations location), we will have to update | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
development tools as well. | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Locations | ||
********* | ||
|
||
Dumps of the translation/localization files from Transifex for the Open edX Release | ||
project already exist in a repository with the name of openedx/openedx-i18n. A new | ||
sarina marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we deprecate this openedx/openedx-i18n repo as part of this OEP too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to get @regisb's input on whether the proposed solution would allow us to also deprecate openedx/openedx-i18n before we commit to removing it in this OEP. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the new openedx-translations repo contains strings for all languages and repos and it follows the regular minor/major release schedule, then yes we will be able to get rid of the openedx-i18n repo. I'm looking forward to it :) |
||
repository named openedx/openedx-translations, will be similarly structured, but contain | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
the translation files for all repositories within openedx. The GitHub Transifex app will | ||
be installed in the openedx organization. Similar to how the Build-Test-Release Working | ||
Group creates a new branch for each new named release of edx-platform, translation | ||
releases can also be kept in branches corresponding to edx-platform releases. | ||
|
||
Rejected Alternatives | ||
********************* | ||
|
||
Rewriting the Current Tooling for the New API | ||
============================================= | ||
|
||
The source code for the edx-transifex-bot is missing. We could rewrite the current | ||
tooling to try to solve the problems encountered in the last two Open edX releases and | ||
upgrade to the new API, but this approach would require a full rewrite, potentially more | ||
expensive than doing the rewrite in a way that Transifex more cleanly supports. It should | ||
also be mentioned that GitHub discourages the use of bots and separate bot accounts; they | ||
strongly recommend using GitHub Apps. | ||
|
||
Making a Transifex Project for each Repository | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
============================================== | ||
|
||
As translation support is provided for more repos, the total cost will also increase. A | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Transifex Project houses the content to be translated and needs to be created before any | ||
content can be added for translation. Transifex Projects can only support 1 GitHub | ||
repository each and need to be maintained separately. Maintaining a Transifex Project | ||
involves adjusting configurations, adding new languages, assigning translators to | ||
projects, or any other miscellaneous irregular tasks that would be time-consuming at a | ||
larger scale. If we adda a Transifex Project, each Transifex Project will need to be | ||
Carlos-Muniz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
maintained separately, making debugging issues or tracking the progress of each Transifex | ||
Project time-consuming. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the Provisional definition and the docs for status changes, I'd recommend:
Updating the status to
Under Review (=> Provisional)
to clarify the intended status upon merge, assuming that status is agreed upon. And,Adding a
Follow-up Work
page to this header as noted in the docs above. This link might also provide some thoughts on how this work will come to be.Additionally, this isn't a blocker to this OEP, but is related to "Follow-up Work". I'm wondering what thought has been given to how companies will need to handle private repos and translations? Maybe a separate Discuss thread could be started for this, but has thought already been given to this? Is the idea that others could easily replicate this pattern and re-use tooling against a private repo of translations? How much work will that involve? Etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your comments @robrap.
If I understand correctly, you want to clarify that the next status after "Under Review" should be "Provisional" and not "Accepted", because it hasn't been vetted and adopted in the platform. This makes sense to me, but I don't think we need to specify this in the document.
I will add a link named “Follow-up Work” to the References section of the OEP header.
I think this is a good first question for the Follow-up Work link. The openedx-atlas CLI tool can pull translation files from any repository that works with git. And the GitHub Action that generates translations and moves them to openedx-translations can also be reappropriated for private repos. The work involved will be: copying the github action that generates the translations to another repository, changing the organization and names of the repos, and attaching it to a separate Transifex project via the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Carlos-Muniz on point 1, @robrap is suggesting that you change the status from:
Under Review
to one of these:
Under Review (=> Provisional)
Under Review (=> Accepted)
in order to clarify whether you want this to merge in as a Provisional or Accepted, which is information the reviewers should know. I agree with him.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside, @robrap : I think this is evidence that the OEP-1 status guidance that we collaborated on could be improved :) OEP-1 presents
Under Review
as a valid status even though we'd really prefer that folks always include the(=> ...)
part.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the clarification. I've changed it.