Write resources without fetch, using data model fields - #4391
Conversation
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
A few high-level notes and a coupe inline.
- I got
ValueError: A @fluent-fn attribute is required for :platformwhen trying to download http://localhost:8000/sl/firefox/browser/browser/aboutLogins.ftl?string=200689 - Does the patch also fix #4107?
- Why do we download single files as ZIP?
- We should also allow for download (of all project resources) from the All Resources view (or is that #2134)?
| .exclude(approved_date__gt=now) # includes approved_date = None | ||
| .select_related("entity") | ||
| ) | ||
| res = build_moz_l10n_resource(db_res) |
There was a problem hiding this comment.
Previously, res = parse_resource(ref_path) was inside a try - catch, so the sync carried on in case of an error. Now it seems like it fails for the whole project.
There was a problem hiding this comment.
I think that's appropriate? The only reasons this should throw are indicators of something being seriously wrong, and we probably should not continue with a sync in that case.
| response = HttpResponse() | ||
| response.content = bytes_io.getvalue() | ||
| response["Content-Type"] = "application/zip" | ||
| zip_name = re.sub(r"[^.]+$", "zip", filename) |
There was a problem hiding this comment.
This is broken if filename has a dot in the middle?
Why not just add the extension?
zip_name = f"{filename}.zip"There was a problem hiding this comment.
This is completely fine with filenames that contain dots; it's only replacing the extension, which is always preceded by a dot. Essentially, it's replicating the default behaviour of gz.
There was a problem hiding this comment.
I need to get better at reading regular expressions :-(
On the naming: would it make more sense to have the extension included though? Currently it produces it_firefox_browser_browser_aboutLogins.zip, maybe it_firefox_browser_browser_aboutLogins_ftl.zip.
|
Tested with my locale instance, this is Firefox
|
|
Can't reproduce with my local instance. Could it be a problem with MF2-related data migrations? |
Co-authored-by: Matjaž Horvat <matjaz.horvat@gmail.com>
This does not replicate for me.
No, downloading source/template files is a separate issue.
No good reason; that's what the now-removed
Yeah, let's address that in a separate PR. |
I'm not seeing that. Have you run a force-sync for |
|
It doesn't match? It serialized day/year/month, I'd expect day/month/year.
I'm positive I did several times, but unfortunately I can't tell from Docker's logs 🤔 I do it frequently to remove empty trailing lines that I add to trigger the production instance sync |
Locally, I'm seeing day/month/year when serialising |
That's dumb, I missed the dependency update in this PR 🤦🏼 Saw this comment only after I started rebuilding the image, and the image is now using 0.14.1. The variable order is fixed mozilla-l10n/firefox-l10n@8a5fb3a But it doesn't look like it restored those group-level comments. EDIT: I reverted the last 2 commits, and added a new forced sync, so the diff against the output of this branch is clearer |
Does the group-level comment show up on your instance for e.g. this string? https://pontoon.mozilla.org/it/firefox/browser/browser/aiFeatures.ftl/?string=337021 |
Yes. 
|
|
For both This is likely a side-effect of the merge from cross-channel. So, this patch is doing the right thing, what's confusing is why these changes don't show up on your side if you test it. |
I think I was seeing a group comment matching the one that was being removed as being included in my local results, and not noticing that the source had two comments with the same contents. |
4 participants
After merging this, mozilla/webservices-infra#9295 should be reverted.
The serialization is changed to rely on data we store during the sync, and on the data model representations of translations, rather than on re-parsing the source resource and patching it with strings.
This will mean that the serialization will change for rather many files, as e.g. standalone comments are getting dropped.
The three commits here could also be filed each as separate stacked PRs; please let me know if that would be preferable.
Direct downloads of translated resources are re-enabled. Their UX is essentially unchanged from the current one, i.e. only single file downloads are offered. We could and should iterate on this to address other pre-existing issues, but as later work.