Roundup Tracker - Issues

Issue 2551184

classification
improve i18n handling
Type: rfe Severity: normal
Components: Database Versions: 2.2.0
process
Status: new
:
: : marcus.priesch, rouilj, schlatterbeck
Priority: :

Created on 2021-12-16 13:37 by marcus.priesch, last changed 2021-12-17 00:41 by rouilj.

Messages
msg7423 Author: [hidden] (marcus.priesch) Date: 2021-12-16 13:37
I am currently facing a performance issue in the rest API ...

through profiling i found out that most time gets consumed by calls to gettext and struct.unpack ...

digging deeper into the code and discussing with ralf we found out that the time gets consumed by calling roundup.cgi.TranslationService.get_translation (takes about 2ms).

which is not much, but ralf calls it in every detector as he wants to have i18n in the detector and there is no way to get it otherwise ... for 25 detectors, this adds a total of 60ms.

digging further into the logic of where get_translation gets called we found out that it's also not working as expected:

mail_gw: the setting in config.ini (mailgw.language) is ignored, uses ENV setting if found, default "en"

client.py: uses setting tracker.language (which, to me, is not obvious - web.language would be wore intuitive, but it's ok for me), browsers language (if enabled in settings), some "@language=code" form field (why?), ENV or "en"

for the rest api it could be optional if one wants translated error messages or speed - using the NullTranslatior i gain another 2ms in speed per request

to sum up what we (ralf and me) suggest as an enhancement:

  put the "get_translation" functionality somewhere in the hyperdb, to have a i18n object there
  initialise it with the NullTranslator (as this merely costs nothing) and 
  change this if needed down the way (be it in the mail_gw, cli or cgi handler)
  add config item rest_no_translation and xmlrpc_no_translation (or maybe own sections for rest and xmlrpc)

this way:

  detectors can access gettext (through db.i18n) and provide localized error messages, 
  web_interface, cli and mail_gw can change the language if needed
  if nothing gets set by the user, we have maximum speed :)

free for discussion .. ;)

regards,
marcus.
msg7424 Author: [hidden] (rouilj) Date: 2021-12-17 00:41
> through profiling i found out that most time gets consumed
> by calls to gettext and struct.unpack ...

What is struct.unpack?

Also I have to admit the translation code is kind of a black
box for me. I have 4 or more tickets that deal with
translation issues that I haven't been able to
apply/understand.

> digging deeper into the code and discussing with ralf we
> found out that the time gets consumed by calling
> roundup.cgi.TranslationService.get_translation (takes
> about 2ms).

This generates a new translation object correct? IIRC it has
disk access every time to scan the locale directories. So
yeah that can eat time.

Is Ralf doing a:

  from roundup.i18n import _

at the top of each of his detectors? Then an _("text to
translate")?

> digging further into the logic of where get_translation
> gets called we found out that it's also not working as
> expected:

> mail_gw: the setting in config.ini (mailgw.language) is
> ignored, uses ENV setting if found, default "en"

Hmm according to config.ini if empty it should use
tracker.language.

    Default locale name for the tracker mail gateway.  If
    this option is not set, mail gateway will use the
    language of the tracker instance.

Is tracker.language set? If tracker.language is not set,
tracker.language is set from the environment (see
below). Maybe that is what you are seeing?

Of course it could be broken.

I don't know why you would have a different locale for the
mail gateway. Perhaps a German speaking country that does
most of it's work with English speaking counties via email?
I don't know if there is a way to determine what language
email is in. If there was it would be possible to implement
a "use_mailer_language" similar to "use_browser_language".
But this is a bit off the point.

> client.py: uses setting tracker.language (which, to me, is
> not obvious - web.language would be wore intuitive, but
> it's ok for me), browsers language (if enabled in
> settings), some "@language=code" form field (why?), ENV or
> "en"

tracker.language is the trackers base language. The web
interface, email etc. all inherit it (or are supposed to).
If language is not set it uses:

  the language is determined by OS environment variable
  LANGUAGE, LC_ALL, LC_MESSAGES, or LANG, in that order of
  preference.

> for the rest api it could be optional if one wants
> translated error messages or speed - using the
> NullTranslatior i gain another 2ms in speed per request

That seems a reasonable tradeoff. Also it could be enabled
by a query parameter "?@language=code" so the REST user has
the ability to change that (say for debugging or something).
(Not sure what an equivalent method would be for xmlrpc.)

I am not sure if things like the names of statuses are translated
using this or not. So that may be a caveat and a reason to request
translation.

> to sum up what we (ralf and me) suggest as an enhancement:
>
>   put the "get_translation" functionality somewhere in the
>   hyperdb, to have a i18n object there

I would attach it to the hyperdb in client.py rather than
making it part of the hyperdb class.

Look for self.db.tx_Source in roundup/cgi/client.py. I think
putting code to look at the config settings and choose

   self.db.i18n = i18n.get_translation
or
   self.db.i18n = i18n.null_translation (or whatever)

would work. I know tx_Source is available to the detectors
so i18n should be available to the detectors when done this
way as well. I am not sure if db is a hyperdb or a
roundupdb object. I think the latter but I don't remember.

However what would be better is to create and cache the
translator in i18n. So it's set up on the first call to:

  from roundup.i18n import _

_ should be initialized on the first import and just returned
afterwards. I can't think of a reason that is not the right thing to
do. That would benefit code all over the roundup base that uses that
idiom. From a glance at the code it seems to do that now but I may be
misinterpreting it or not understand how this stuff works (likely).

>   initialise it with the NullTranslator (as this merely
>   costs nothing) and change this if needed down the way
>   (be it in the mail_gw, cli or cgi handler)
>
>   add config item rest_no_translation and
>   xmlrpc_no_translation (or maybe own sections for rest
>   and xmlrpc)

I wold prefer two booleans:

  translate_rest = yes 
  translate_xmlrpc = yes 

Otherwise you have a double negative which makes my head
hurt.

If they are set to no, then does it make sense to have the
web code in client.py override db.i18n before it calls the
rest and xmlrpc handlers?

I don't think we need new sections yet.

> this way:

>   detectors can access gettext (through db.i18n) and
>     provide localized error messages, web_interface, cli and
>     mail_gw can change the language if needed
>   if nothing gets set by the user, we have maximum speed :)

That seems reasonable.

-- rouilj
History
Date User Action Args
2021-12-17 00:41:26rouiljsetnosy: + rouilj
messages: + msg7424
2021-12-16 13:37:21marcus.prieschcreate