Page MenuHomePhabricator

i18n is broken on Heritage project after moving to Kubernetes backend.
Closed, ResolvedPublic

Description

On https://tools.wmflabs.org/heritage/api/index.php, I only get the string IDs, not their values (even in English) − eg 'search-monuments-database'

Event Timeline

JeanFred triaged this task as High priority.Jul 3 2016, 3:24 PM

What do you think @Lokal_Profil ? Can you reproduce?

I can confirm that I'm seeing the same thing.
Seems recent though since I was looking on this page when working on some other patches...

Also the html list output for the api looks OK so I guess one of the index page patches messed things up. will investigate

Looks OK in my docker with master checked out.

The likely culprits are either patch 296795 or 297190.

My money is on $I18N not being properly defined based on some ad-hoc testing on the server. But then I might just have tested it wrong ;)

OK so as part of https://gerrit.wikimedia.org/r/#/c/297526/ I investigated this a bit.

Identification of language and available messages works fine.
$I18N->msg( 'msg-id' ) gives the desired result but _( 'msg-id' ) doesn't. This is despite us setting globalfunctions = true. I'm guessing the shortcut clashes with something else already defined somewhere since Intuition checks function_exists( '_' ) before defining its own.

I now see this happening on https://tools.wmflabs.org/heritage/toolbox/ as well. The error _() expects exactly 1 parameter, 2 given makes it very clear that _() is not the function we are expecting to get.

Unless either of you (@Krinkle, @JeanFred) have any other recommendations I would simply replace _() by $I18N->msg() throughout the whole repo.

I believe Heritage's PHP environment may've changed recently. Specifically, the gettext extension is now installed - which means a function named _ already exists and Intuition cannot re-define it.

See php.net/_ and php.net/function.gettext

This would make sense because that function indeed takes one parameter (not two). And it's default behaviour is to return the $message parameter as-is. (Whereas Intuition would return bracketed [$message]).

I'd recommend removing the globalfunctions => true option in heritage:/api/common.php and using the Intuition methods directly (see intuition:/Function.php).

Thanks for the clarification. Yes we recently bumped the php version of heritage so that is likely it.

I'll get on this in the next few days.

Thanks for the clarification. Yes we recently bumped the php version of heritage so that is likely it.

On July 6, @yuvipanda moved the Heritage webservice to the Kubernetes backend. I noticed the i18n issue just afterwards, but could not be 100% sure it was working just before − I really should have just asked Yuvi to revert to see if the problem was there :-/

I think to recall Yuvi say that the difference was Kubernetes was running PHP 5.6. This would explain why we suddenly have this new extension enabled.

So going forward, either we replace all of our i18n, or we revert back to the gridengine backend.

JeanFred renamed this task from i18n appears broken on Heritage API to i18n is broken on Heritage project after moving to Kubernetes backend..Jul 9 2016, 1:42 PM

I'd highly appreciate it if we kept this on k8s - I think that's best for the long term :) I too would go with @Krinke's reccomendation of:

I'd recommend removing the globalfunctions => true option in heritage:/api/common.php and using the Intuition methods directly (see intuition:/Function.php).

If you do want to revert back though, you can do webservice --backend=kubernetes stop and webservice --backend=gridengine start.

Change 298177 had a related patch set uploaded (by Lokal Profil):
Deprecate use of _() as shortcut for $I18N->msg()

https://gerrit.wikimedia.org/r/298177

Change 298177 had a related patch set uploaded (by Lokal Profil):
Deprecate use of _() as shortcut for $I18N->msg()

https://gerrit.wikimedia.org/r/298177

I simply defined _i18n() and _html() as local functions replacing the _() and _html() in intuition and set globalfunctions => false.

I also set it up so that a warning is raised if these are already defined (pre-empting the next time this occurs).

Change 298177 merged by jenkins-bot:
Deprecate use of _() as shortcut for $I18N->msg()

https://gerrit.wikimedia.org/r/298177

Mentioned in SAL [2016-07-13T09:08:43Z] <Lokal_Profil> Deployed latest from Git, 38363cb, 6117b13 (T139267), 96af0dc, 20d6da6