Closed Bug 875313 Opened 11 years ago Closed 11 years ago

rework deployment process to avoid localized strings errors

Categories

(Input :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: willkg, Assigned: willkg)

References

Details

(Whiteboard: u=dev c=general p=3 s=input.2013q3)

It's really important for pages in production to not kick up HTTP 500 errors. One common cause of errors we're having right now is localized strings containing Python tokens that are either typoed, misformatted or not in the original string.

We want to eradicate that possibility.

This bug covers changing the deployment process:

1. remove the svn update from the deployment process
2. pull in the localized strings and compiled .mo files into git
3. document the new strings update process:

   3.1. pull down the latest strings from svn
   3.2. lint them to make sure they're ok
   3.3. any .po files that are error-free get checked into git
   3.4. notify localizers about errors
   3.5. deploy the new error-free strings to production

4. elect someone to be responsible for updating strings in git on a regular basis


One big problem with this is that if the person responsible for updating strings doesn't update them, then they don't get updated. However, we had that problem before--if no one did a deployment, then strings didn't get updated. I think this change doesn't change the situation much.
This is something we should do soonish. Setting priority and whiteboard accordingly.
Priority: -- → P1
Whiteboard: u=dev c=general p= s=input.2013q2
Bumping to 2013q3 since that's where I'm doing this work.

Grabbing this to work on.
Assignee: nobody → willkg
Whiteboard: u=dev c=general p= s=input.2013q2 → u=dev c=general p= s=input.2013q3
Here's where I'm at:

1. I spent a bunch of personal time writing a tool called dennis.

2. dennis can "lint" .po files looking for errors where the translated string has tokens the original string does not have which causes the site to kick up errors.

3. during the deployment process, we have a step where we compile the .po files to .mo files.

My current idea is to change that step so that the deployment script lints the file and if it has no errors in it, then compiles it.

Pros:

1. It's a very minor change to the deployment process.
2. It should be pretty easy to implement.
3. It won't require IT to change anything.
4. It is almost exactly like the current process so future Input devs won't be surprised by additional maintenance tasks they weren't aware of.

Cons:

1. There's nothing in the process that tells anyone about the actual problems encountered. So files with problems can stay problematic ad infinitum.

I'm going to defer the major con until later and proceed with testing this plan out.
To clarify, the plan in comment #3 is different than the plan suggested in the original description. I'm ditching the plan in the description for the plan in comment #3 which has fewer issues.
Most of the work is in this PR: https://github.com/mozilla/fjord/pull/126

I need to test some things by deploying on stage. I also need to make the changes to the update script.
I pushed the changes to stage. The linting-and-compiling step prints all the data to /media/postatus.txt. You can see that here:

https://input.allizom.org/media/postatus.txt

I sent an email to Milos about this.
Copying the important bits of the email here and flagging Milos with a needsinfo?.

===

I've finished up the changes and have them on Input stage now. I'm asking for feedback before proceeding to push these changes to production.

The gist of the changes are these:

1. I wrote a linting command that goes through a .po file and looks for errors that will cause Input to throw an HTTP 500. If it finds errors, then the .po file won't be compiled to an .mo file. Thus updates to .po files that include errors will no longer make it to production.

2. The linting command shows all the output for errors it discovered at /media/postatus.txt so translators can at any time see what the current status of things is and why their translation updates aren't getting to production.

You can see the output on stage now:

https://input.allizom.org/media/postatus.txt

There's a list of the files that have errors and are thus busted at the end.

3. This is backwards compatible with the current system. Any .mo files that are in production now will continue to be in production. This only affects any new .po changes.


My questions to you:

Does this system sound ok?

How do you want to go forward with the litany of errors that exist now? I can edit the .po files in svn and wipe out any translated strings that have errors. I can't use Verbatim because I don't have access to make changes there.
Flags: needinfo?(milos)
I haven't heard anything from Milos, so my plan is as follows:

1. land the changes
2. look at the lint output and delete the strings that have errors in them and commit all that to svn


That'll cause a bunch of the site to show up in English for locales which sucks, but having those pages error out is worse.

I'll probably do this later this week.
Whiteboard: u=dev c=general p= s=input.2013q3 → u=dev c=general p=3 s=input.2013q3
PR: https://github.com/mozilla/fjord/pull/129

Landed in master in:

https://github.com/mozilla/fjord/commit/db72b58
https://github.com/mozilla/fjord/commit/d5d02b5
https://github.com/mozilla/fjord/commit/033905c
https://github.com/mozilla/fjord/commit/7acbc45

I also fixed all the strings that were erroring out.

(fjord) (M=ef716 ../) saturn ~/mozilla/fjord/locale> svn log | head
------------------------------------------------------------------------
r118446 | wkahngreene@mozilla.com | 2013-07-29 16:45:16 -0400 (Mon, 29 Jul 2013) | 1 line

fixed last of the errors from linting
------------------------------------------------------------------------
r118445 | wkahngreene@mozilla.com | 2013-07-29 16:36:57 -0400 (Mon, 29 Jul 2013) | 1 line

fix more errors from linting
------------------------------------------------------------------------
r118444 | wkahngreene@mozilla.com | 2013-07-29 16:02:50 -0400 (Mon, 29 Jul 2013) | 1 line


Going forward we will no longer push .po changes that have errors in them. Localizers can look at the postatus file in production to see whether there are issues.


Milos: You'll need to update the strings in Verbatim since I made changes directly to the .po files. Also, if this doesn't work for you, let me know.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Please don't try to check in to SVN. As I see, you just did. That's not good.

I'm confused where those reports get their strings from, tbh. Can you point to sources that are affected?

Generally speaking, changing the l10n infrastructure without anyone from l10n commenting on it is asking for bustage on other sides.
Flags: needinfo?(milos)
(In reply to Axel Hecht [:Pike] from comment #10)
> Please don't try to check in to SVN. As I see, you just did. That's not good.

I agree. I was told Milos was my point of contact for l10n stuff. I've been trying to get him to chime in for a couple of months now on several issues. It's not ok for me to be waiting on someone who never replies to email, irc pings, needsinfo or bugmail.

As I understand it, all he needs to do is update Verbatim with the changes I made.


> I'm confused where those reports get their strings from, tbh. Can you point
> to sources that are affected?

I don't understand this question.


> Generally speaking, changing the l10n infrastructure without anyone from
> l10n commenting on it is asking for bustage on other sides.

I totally agree. I need a point of contact who responds. Can I rely upon you for future Input l10n issues?
Take for example this file for French (a locale usually up to date)
http://viewvc.svn.mozilla.org/vc/projects/l10n-misc/trunk/input/locale/fr/LC_MESSAGES/messages.po?r1=118444&r2=118443&pathrev=118444

Those strings you removed are marked as fuzzy. Does the compilation of .po files include fuzzy strings? I don't think so, but I could be wrong. If I'm not, your tool has a lot of false positives.

As a side note, the system needs a more clear solution to warn people of errors.
You have the email of the last localizer in the header, you could send a message out. Or even better you can create a bug in the locale component. 
I honestly think that checking a hidden .txt file for errors is not an option.
(In reply to Francesco Lodolo [:flod] from comment #12)
> Take for example this file for French (a locale usually up to date)
> http://viewvc.svn.mozilla.org/vc/projects/l10n-misc/trunk/input/locale/fr/
> LC_MESSAGES/messages.po?r1=118444&r2=118443&pathrev=118444
> 
> Those strings you removed are marked as fuzzy. Does the compilation of .po
> files include fuzzy strings? I don't think so, but I could be wrong. If I'm
> not, your tool has a lot of false positives.

I don't understand why there's junk in the .po file. Is that a common problem of Verbatim?


> As a side note, the system needs a more clear solution to warn people of
> errors.
> You have the email of the last localizer in the header, you could send a
> message out. Or even better you can create a bug in the locale component. 
> I honestly think that checking a hidden .txt file for errors is not an
> option.

That's what I was getting at with comment #7 which I never got an answer to. I totally agree the current situation isn't great, but I can't guess what will work for you and what won't.

Regarding emailing people, I don't think I can do that from the chief node during deployments, so that's probably a no-go.

I'm game for creating bugs by hand. Walk me through what "create a bug in the locale component" entails.

To clarify, I'm really sorry we're in this state. I don't want to be a pain in the ass for l10n folks. It's frustrating as all hell for me. I absolutely want to work out the kinks for this new process because we're going to roll it out for Kitsune/SUMO, too.
If someone doesn't respond, escalate, and I or chofmann would be the guys to escalate to.

I can't help with any of the actual work, but if there's a resourcing problem, I can only help with that if I know about it. I'll take these offline, ftr.

To the actual substance: If development teams change their l10n process, we can't support them. This affects input right now, but also AMO. It might turn out that we actually can, but we can't commit that tools we don't own like pootle support processes we don't know about.

Regarding the test results, I'm surprised to see those kind of results, and I failed to find "Most recent message" in any place I looked. Thus I can't really make any educated comments if the surprises in those are on the l10n side, or if the tool you did created false positives. If you have pointers to those strings in en-US and l10n, I could take a closer look.
(In reply to Axel Hecht [:Pike] from comment #14)
> If someone doesn't respond, escalate, and I or chofmann would be the guys to
> escalate to.
> 
> I can't help with any of the actual work, but if there's a resourcing
> problem, I can only help with that if I know about it. I'll take these
> offline, ftr.

That helps a ton. Thank you!


> To the actual substance: If development teams change their l10n process, we
> can't support them. This affects input right now, but also AMO. It might
> turn out that we actually can, but we can't commit that tools we don't own
> like pootle support processes we don't know about.

The only thing I did was change the deployment scripts so they don't compile .po files to .mo files if they have errors which would cause our production systems to kick up HTTP 500 errors. We can't continue to have fires in production preventing users from using the site for days on end. Milos suggested we make this change a while back.

I don't think that constitutes a major change in l10n process that should cause issues with the rest of the l10n system.

I don't expect you to support the tool I wrote, so that's fine.

And to clarify, I don't plan on making more svn changes. This change to deployments makes it so I don't have to put out fires in production anymore, so I no longer have to do anything in svn except the occasional extract/merge strings.


> Regarding the test results, I'm surprised to see those kind of results, and
> I failed to find "Most recent message" in any place I looked. Thus I can't
> really make any educated comments if the surprises in those are on the l10n
> side, or if the tool you did created false positives. If you have pointers
> to those strings in en-US and l10n, I could take a closer look.

I don't understand why strings are in the .po files that aren't in the .pot file or why there's all this stuff commented out. Not a big deal--just weird.

Regardless, I tweaked dennis so it should report far fewer false positives. I'll continue to hone it as other false positive scenarios come up.
(In reply to Will Kahn-Greene [:willkg] from comment #13)
> I don't understand why there's junk in the .po file. Is that a common
> problem of Verbatim?

Not really familiar with the process, but I think that's more about the merge process than Verbatim.

> I'm game for creating bugs by hand. Walk me through what "create a bug in
> the locale component" entails.

Mozilla Localizations -> locale code, and I think you can use Bugzilla's APIs for that. But better to talk with pike about this.
You need to log in before you can comment on or make changes to this bug.