Refactor sync #3312

eemeli · 2024-09-02T14:38:20Z

Fixes #2057
Fixes #2078
Closes #2083 -- source changes are now sync'd to targets eagerly
Fixes #2087 -- git file moves are caught, but not copies
Closes #2129 -- refactoring the sync changes the performance characteristics completely
Fixes #2169
Fixes #2175
Fixes #2189
Fixes #2211
Fixes #2242
Closes #2285 -- not relevant after the refactor
Fixes #2641
Fixes #3302
Fixes #3449
Fixes #2154

This is effectively a rewrite of the sync_project() function that's currently here, and which ends up calling most of the code under pontoon/sync/.

The end results of the code here should be the same as currently, but the implementation is completely new, and does things in a different order.

Per-locale repositories are dropped here, as per #3303.

Explicitly left out of this PR but liable to change later:

What sync logging information is persisted in the database.
How files are read and written.
How localizable messages are represented in the database.
How aggregated stats are gathered.

mathjazz

Very high-level look, but this already looks very promising!

Currently, Pretranslation is tightly integrated with sync in order to minimize time between exposing new strings for localization and pretranslating them. What's your plan with this?

pontoon/base/models/changed_entity_locale.py

pontoon/base/models/project.py

pontoon/sync/sync_project.py

pontoon/sync/checkouts.py

pontoon/sync/sync_entities_from_repo.py

pontoon/sync/sync_translations_to_repo.py

eemeli · 2024-09-04T22:08:30Z

Currently, Pretranslation is tightly integrated with sync in order to minimize time between exposing new strings for localization and pretranslating them. What's your plan with this?

Ah, I'd missed that! Yeah, that needs to happen the same as before.

…object

requirements/default.txt

codecov-commenter · 2024-10-01T15:26:11Z

Codecov Report

Attention: Patch coverage is 90.05266% with 170 lines in your changes missing coverage. Please review.

Project coverage is 79.20%. Comparing base (e3e7168) to head (ff2b00d).
Report is 9 commits behind head on main.

Additional details and impacted files

eemeli · 2024-10-01T15:47:30Z

This is now at the dangerous stage of looking like it works. But some verification work still remains:

Add tests for the fixtures used by the old sync tests.
Check the sync results against the active pontoon.mozilla.org projects.

pontoon/base/__init__.py

pontoon/base/views.py

pontoon/sync/core/__init__.py

mathjazz · 2024-12-12T14:36:40Z

Pushed the latest version to stage and the download from Pontoon Test now works. 👍

Firefox times out for now.

If we want to permanently Download all projects for all files, we should also show Download Translations entry in the Profile menu in the All Resources view. Not necessarily as part of this PR.

And, we should drop "Download prefix or path to TOML file" from the Admin, DB and the docs.

flodolo · 2024-12-12T14:42:43Z

Firefox times out for now.

It works for me locally, but it takes ~20s, so not surprised it times out on Heroku.

…gated stats

eemeli · 2024-12-13T11:08:09Z

I managed to download AMO file (in 2nd attempt). I made a change to one of the strings and uploaded it. That stats went off.

After some investigation, the stats "went off" already during the preceding sync, not due to the upload.

This should now be fixed, by making the total_strings counts in aggregated stats depend on the locale's plural category count for gettext resources with plural messages.

Most of the work to effect that went into cleaning up the optimizations introduced in #1140, and making the tests it introduced work with more real-world usage patterns. The update_stats argument of Translation.save() is dropped, as it was only used to test a code path that is only ever called from said tests.

It would be a good idea to call ./manage.py calculate_stats once this lands, rather than waiting for each project's first sync to fix the stats.

eemeli · 2024-12-13T12:23:59Z

@mathjazz I think I've now addressed all comments, PTAL?

mathjazz · 2024-12-13T13:32:10Z

The app crashes when I try to download a file from the Firefox project.

This still happens.

I managed to download AMO file (in 2nd attempt). I made a change to one of the strings and uploaded it. That stats went off.

This is fixed! 👍

I've downloaded translations from the pontoon-test-1 project and uploaded amo.po back and the changes I made locally were reflected correctly (incl. the stats). But: All fuzzy strings got rejected. I checked locally on master and it works as expected there.

flodolo · 2024-12-13T15:58:58Z

Sync is failing for me locally, while it's been working fine until yesterday (just updated the project, translated one string, and tried a sync)

[INFO:pontoon.sync.core] 2024-12-13 16:35:22,941 [firefox-local] Sync start
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:22,945 Git: Updating repo https://github.com/mozilla-l10n/firefox-l10n-source.git
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:22,952 Git: Repo updated.
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:22,955 Git: Branch update checked out.
[INFO:pontoon.sync.core.checkout] 2024-12-13 16:35:22,957 [firefox-local] Repo at 7fd0da7a2
[DEBUG:pontoon.sync.core.checkout] 2024-12-13 16:35:22,958 [firefox-local] source root: /app/media/projects/firefox-local/mozilla-l10n/firefox-l10n-source.git
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:22,958 Git: Updating repo [email protected]:mozilla-l10n/firefox-l10n.git
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:23,074 Git: Repo updated.
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:23,158 Git: Branch main checked out.
[INFO:pontoon.sync.core.checkout] 2024-12-13 16:35:23,159 [firefox-local] Repo updated from 89d1f8cda72 to dafff26cecf
[DEBUG:pontoon.sync.core.checkout] 2024-12-13 16:35:23,160 [firefox-local] target root: /app/media/projects/firefox-local/[email protected]:mozilla-l10n/firefox-l10n.git
[DEBUG:pontoon.sync.core.paths] 2024-12-13 16:35:36,935 [firefox-local] Paths(auto): ref_root=. base=../../[email protected]:mozilla-l10n/firefox-l10n.git
[INFO:pontoon.sync.core.translations_from_repo] 2024-12-13 16:35:36,939 [firefox-local] Reading changes from 3 changed target files
[DEBUG:pontoon.sync.core.translations_from_repo] 2024-12-13 16:35:36,939 [firefox-local] Scanning for translation updates...
[DEBUG:pontoon.sync.core.translations_from_repo] 2024-12-13 16:35:37,026 [firefox-local] Filtering matches from translations...
[INFO:pontoon.sync.core.translations_to_repo] 2024-12-13 16:35:37,033 [firefox-local] Updating 2 changed resources
[INFO:pontoon.sync.core.translations_to_repo] 2024-12-13 16:35:37,033 [firefox-local:browser/browser/browserSets.ftl] Updating locales: it
[DEBUG:pontoon.sync.formats.ftl] 2024-12-13 16:35:37,056 Saving file: /app/media/projects/firefox-local/[email protected]:mozilla-l10n/firefox-l10n.git/it/browser/browser/browserSets.ftl
[INFO:pontoon.sync.core.translations_to_repo] 2024-12-13 16:35:37,057 [firefox-local:browser/browser/sidebar.ftl] Updating locales: it
[DEBUG:pontoon.sync.formats.ftl] 2024-12-13 16:35:37,085 Saving file: /app/media/projects/firefox-local/[email protected]:mozilla-l10n/firefox-l10n.git/it/browser/browser/sidebar.ftl
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 15:56:41,106 Git: Commit to repository.
[WARNING:pontoon.sync.core.translations_to_repo] 2024-12-13 15:56:42,570 [firefox-local] git commit failed: Remote contains work that you do not have locally. To github.com:mozilla-l10n/firefox-l10n.git
 ! [rejected]                main -> main (fetch first)
error: failed to push some refs to 'github.com:mozilla-l10n/firefox-l10n.git'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

[ERROR:pontoon.base.errors] 2024-12-13 15:56:42,580 Remote contains work that you do not have locally. To github.com:mozilla-l10n/firefox-l10n.git
 ! [rejected]                main -> main (fetch first)
error: failed to push some refs to 'github.com:mozilla-l10n/firefox-l10n.git'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/celery/app/trace.py", line 453, in trace_task
    R = retval = fun(*args, **kwargs)
                 ^^^^^^^^^^^^^^^^^^^^
  File "/app/pontoon/sync/tasks.py", line 44, in sync_project_task
    sync_project(project, sync_log, pull=pull, commit=commit, force=force)
  File "/app/pontoon/sync/core/__init__.py", line 69, in sync_project
    sync_translations_to_repo(
  File "/app/pontoon/sync/core/translations_to_repo.py", line 82, in sync_translations_to_repo
    raise error
  File "/app/pontoon/sync/core/translations_to_repo.py", line 78, in sync_translations_to_repo
    repo.commit(co.path, commit_msg, commit_author, co.repo.branch, co.url)
  File "/app/pontoon/sync/repositories/git.py", line 93, in commit
    raise CommitToRepositoryException(error)
pontoon.sync.repositories.utils.CommitToRepositoryException: Remote contains work that you do not have locally. To github.com:mozilla-l10n/firefox-l10n.git
 ! [rejected]                main -> main (fetch first)
error: failed to push some refs to 'github.com:mozilla-l10n/firefox-l10n.git'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

eemeli · 2024-12-16T11:48:58Z

@flodolo Based on the log and these lines in particular:

[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:22,958 Git: Updating repo [email protected]:mozilla-l10n/firefox-l10n.git
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:23,074 Git: Repo updated.
[DEBUG:pontoon.sync.repositories.git] 2024-12-13 16:35:23,158 Git: Branch main checked out.
[INFO:pontoon.sync.core.checkout] 2024-12-13 16:35:23,159 [firefox-local] Repo updated from 89d1f8cda72 to dafff26cecf

As the commit dafff26cecf isn't on GitHub, I'm pretty sure that you have a local config that explicitly sets the repo branch to main, rather than leaving that field empty, and that you had some local changes committed to the main branch before the sync. Those changes should've been dismissed, but some of the code is run in the wrong order, so a reset that should follow a checkout is run before it.

This is technically an unrelated bug to the sync refactor, but I've fixed it here.

…if they stay fuzzy

eemeli · 2024-12-16T16:22:51Z

The app crashes when I try to download a file from the Firefox project.

As a workaround, downloads are now handled by redirecting the request to the target repository. For GitHub & GitLab the target is the raw version of the current resource.

Because we currently only store the source path and not the target path in the database, this still requires an initial git clone or pull.

… source & target repos

flodolo · 2024-12-17T07:29:26Z

Different problem

[INFO:pontoon.sync.core] 2024-12-17 07:09:57,777 [firefox-local] Sync start
[DEBUG:pontoon.sync.repositories.git] 2024-12-17 07:09:57,779 Git: Updating repo https://github.com/mozilla-l10n/firefox-l10n-source.git
[DEBUG:pontoon.sync.repositories.git] 2024-12-17 07:09:58,381 Git: Repo updated.
[DEBUG:pontoon.sync.repositories.git] 2024-12-17 07:09:58,385 Git: Branch update checked out.
[INFO:pontoon.sync.core.checkout] 2024-12-17 07:09:58,393 [firefox-local] Repo updated from 444fda616 to eceb2cee5
[DEBUG:pontoon.sync.core.checkout] 2024-12-17 07:09:58,394 [firefox-local] source root: /app/media/projects/firefox-local/mozilla-l10n/firefox-l10n-source.git
[DEBUG:pontoon.sync.repositories.git] 2024-12-17 07:09:58,394 Git: Updating repo [email protected]:mozilla-l10n/firefox-l10n.git
[DEBUG:pontoon.sync.repositories.git] 2024-12-17 07:10:00,069 Git: Repo updated.
[DEBUG:pontoon.sync.repositories.git] 2024-12-17 07:10:00,163 Git: Branch main checked out.

The source repo was correctly updated, but no new strings were exposed for localization (there are quite a few in this update).
mozilla-l10n/firefox-l10n-source@2da0d05
mozilla-l10n/firefox-l10n-source@eceb2ce

EDIT: even running manage.py sync_projects with --force didn't fix it. Doing it on main branch discovered the 5 strings.

eemeli · 2024-12-17T11:45:45Z

The source repo was correctly updated, but no new strings were exposed for localization

After some investigation, this issue was identified as having been caused by first running a sync from this branch, then main, then this branch again. That sequence of actions leaves a templates/ subdirectory with a second clone of the source repo under media/projects/firefox/, which was being mis-detected as the reference root.

This problem will not arise once the PR is merged.

mathjazz

Tested on stage one more time and didn't spot any issues.

Syncing a month worth of Firefox changes took much less time than with the current code, and I haven't seen a single instance of "Memory quota exceeded" message in the logs.

Well done!

Left some comments inline, but nothing major. We should file a few bugs.

Also, we need to update the README:
https://github.com/mozilla/pontoon/blob/main/pontoon/sync/README.md

docs/user/localizing-your-projects.rst

mathjazz · 2024-12-19T11:20:58Z

pontoon/base/models/locale.py

@@ -390,6 +390,7 @@ def stats(self):
            {
                "title": "all-resources",
                "resource__path": [],
+                # FIXME rename as total_strings


File a good first issue for this?

pontoon/sync/core/translations_from_repo.py

pontoon/sync/core/translations_to_repo.py

pontoon/sync/utils.py

Add new sync implementation (WIP, untested)

3d4c761

mathjazz reviewed Sep 4, 2024

View reviewed changes

eemeli added 2 commits September 9, 2024 15:03

Address code review comments, start adding tests

9e65c9a

Merge branch 'main' into sync-refactor

3d6a1ec

eemeli mentioned this pull request Sep 16, 2024

Path improvements for use in Pontoon mozilla/moz-l10n#22

Merged

Add integration tests for new code; fix discovered issues

397e5f5

eemeli force-pushed the sync-refactor branch from 1f24718 to 397e5f5 Compare September 16, 2024 15:57

eemeli added 5 commits September 17, 2024 13:56

Update requirements

5778090

Satisfy lint

7f3bfcb

Call repo commit() with pre-formatted author string rather than User …

83b8fd7

…object

Add end-to-end test

3005870

Add task wrapper & force option

5b227e9

flodolo reviewed Sep 22, 2024

View reviewed changes

requirements/default.txt Outdated Show resolved Hide resolved

eemeli added 9 commits September 22, 2024 09:00

Refactor handle_upload_content() into sync_uploaded_file()

e657223

Replace get_download_content() with download_translations_zip()

5da483f

Remove old sync implementation

2f0ea10

Pretranslate added & changed resources, fix task invocations

bb97199

Move & rename pontoon.sync contents around, adding pontoon.sync.core

3da84c0

Combine upload & download functions into pontoon.sync.utils

92fb993

Merge branch 'main' into sync-refactor

410a278

Include --no-strip-extras in uv pip compile calls

83049d9

Satisfy ruff

f92e331

File format detector cleanup, drop unused template

fcfc5fa

eemeli commented Oct 1, 2024

View reviewed changes

pontoon/base/__init__.py Show resolved Hide resolved

eemeli commented Oct 1, 2024

View reviewed changes

pontoon/base/views.py Outdated Show resolved Hide resolved

eemeli commented Oct 1, 2024

View reviewed changes

pontoon/sync/core/__init__.py Show resolved Hide resolved

Fix last_synced_revision data, drop remaining multi_locale references

15ef5b6

Fix total_strings counts to depend on gettext locale plurals in aggre…

c309617

…gated stats

eemeli mentioned this pull request Dec 13, 2024

Drop permalink_prefix repo field as unused #3489

Open

Drop unused ResourceQuerySet

c239d7d

Dismiss local git repo edits when branch is specified

784dec2

eemeli added 2 commits December 16, 2024 15:52

During update from repo, keep previously fuzzy suggestions unchanged …

8f26503

…if they stay fuzzy

Rather than creating zip, "download" by redirecting to target repository

3762da5

Add shortcut (read: hack) for downloading from projects with separate…

74e4a14

… source & target repos

eemeli added 2 commits December 17, 2024 19:21

Include active fuzzy translations when writing to repo

357f7fe

When approving matching prior translations, do not also reject them

008fa56

eemeli mentioned this pull request Dec 18, 2024

Removing a fuzzy string in the repository is undone during the next sync #3498

Open

eemeli added 2 commits December 18, 2024 16:51

Use locale's total_strings for GET <locale>/<slug>/parts/

ff95768

Count translation updates before dropping approvals from the dict

c931f71

mathjazz approved these changes Dec 19, 2024

View reviewed changes

flodolo approved these changes Dec 19, 2024

View reviewed changes

mathjazz mentioned this pull request Dec 19, 2024

Prevent mass loss of translation metadata during sync #3500

Open

Rename add_errors() as add_failed_checks()

ff2b00d

eemeli merged commit 42421cd into mozilla:main Dec 19, 2024
2 checks passed

eemeli deleted the sync-refactor branch December 19, 2024 22:09

This was referenced Dec 20, 2024

Update sync README #3501

Open

Prevent one project from blocking all others to sync #2507

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor sync #3312

Refactor sync #3312

eemeli commented Sep 2, 2024 •

edited by mathjazz

Loading

mathjazz left a comment

eemeli commented Sep 4, 2024

codecov-commenter commented Oct 1, 2024 •

edited

Loading

eemeli commented Oct 1, 2024 •

edited

Loading

mathjazz commented Dec 12, 2024

flodolo commented Dec 12, 2024

eemeli commented Dec 13, 2024

eemeli commented Dec 13, 2024

mathjazz commented Dec 13, 2024

flodolo commented Dec 13, 2024 •

edited

Loading

eemeli commented Dec 16, 2024

eemeli commented Dec 16, 2024

flodolo commented Dec 17, 2024 •

edited

Loading

eemeli commented Dec 17, 2024

mathjazz left a comment •

edited

Loading

mathjazz Dec 19, 2024

Refactor sync #3312

Refactor sync #3312

Conversation

eemeli commented Sep 2, 2024 • edited by mathjazz Loading

mathjazz left a comment

Choose a reason for hiding this comment

eemeli commented Sep 4, 2024

codecov-commenter commented Oct 1, 2024 • edited Loading

Codecov Report

eemeli commented Oct 1, 2024 • edited Loading

mathjazz commented Dec 12, 2024

flodolo commented Dec 12, 2024

eemeli commented Dec 13, 2024

eemeli commented Dec 13, 2024

mathjazz commented Dec 13, 2024

flodolo commented Dec 13, 2024 • edited Loading

eemeli commented Dec 16, 2024

eemeli commented Dec 16, 2024

flodolo commented Dec 17, 2024 • edited Loading

eemeli commented Dec 17, 2024

mathjazz left a comment • edited Loading

Choose a reason for hiding this comment

mathjazz Dec 19, 2024

Choose a reason for hiding this comment

eemeli commented Sep 2, 2024 •

edited by mathjazz

Loading

codecov-commenter commented Oct 1, 2024 •

edited

Loading

eemeli commented Oct 1, 2024 •

edited

Loading

flodolo commented Dec 13, 2024 •

edited

Loading

flodolo commented Dec 17, 2024 •

edited

Loading

mathjazz left a comment •

edited

Loading