Roundup Tracker - Issues

Issue 2551184

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

Created on 2021-12-16 13:37 by marcus.priesch, last changed 2023-07-21 00:33 by rouilj.

Files
File name Uploaded Description Edit Remove
improve_i18n.patch marcus.priesch, 2022-04-27 15:50
improve_i18n_2.patch marcus.priesch, 2022-04-28 08:37
improve_i18n_3.patch marcus.priesch, 2022-04-28 15:34
improve_i18n_4.patch marcus.priesch, 2022-04-29 10:11
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
msg7459 Author: [hidden] (rouilj) Date: 2022-03-06 03:19
Hi Marcus:

Have you had any more ideas/code on this?

-- rouilj
msg7462 Author: [hidden] (marcus.priesch) Date: 2022-03-22 08:21
Hello John,

no, i am sorry, i was busy with other tasks, but will take a look at it 
soon.

regards,
marcus.
msg7469 Author: [hidden] (marcus.priesch) Date: 2022-04-27 15:50
Hi John,

finally i have a patch to test with lots of changes ;)

but first some comments on your mail:

> What is struct.unpack?

it is used internally in parsing of the .mo files - i am not sure why it
is doing this ... but thats for another story.

> 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 _

no, he was doing a

_ = roundup.cgi.TranslationService.get_translation \
     (db.config.TRACKER_LANGUAGE, db.config.TRACKER_HOME).gettext

which does what you state above and was run in every detector which got
loaded - and there are a lot ;)

if you want to have translated error messages from during a web request,
than you cant simply do a

	from roundup.i18n import _

because the language can be different for every request see
"use_browser_language"

you also dont get the locales from tracker_home by just doing

	from roundup.i18n import _

thats also the reason why we want the "translator" attached to the db so
that we always have the correct translator at hand (including 
tracker_home locales and users lang preferences)

>> 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.

yes it was broken - nobody used MAIL_LANGUAGE in the code ;)

> 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.

i am also not aware of such a machanism, but maybe you set up the
mailgw on an "english" system and want the mailgw (and the tracker)
run for german users - without having to change the locale of the
whole system - i dont know ;)

it's documented, but it behaves wrong, so i fixed it ;)

> tracker.language is the trackers base language. The web
> interface, email etc. all inherit it (or are supposed to).
[...]

>> 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.)

it is implemented exactly the same for rest and xmlrpc (the form needs
to have a @language field) i am not in xmlrpc, therefore i cannot say
much about it's behaviour in xmlrpc.

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

so here is what i have done:

actually a NullTranslator is attached to the db in
roundupdb.Database.__init__ which gets called in rbms_common and back_anydbm

i am not sure if the "global _" thing is correct as i implemented it so
this needs check and test.

in Client.py this gets overridden when TRANSLATE_REST|XMLRPC is set,
considering "USE_BROWSER_LANGUAGE" and TRACKER_LANGUAGE/Environment.

in the MailGW it gets set either to MAIL_LANGUAGE or TRACKER_LANGUAGE
or defaults to Env

in cgi.engine_zopetal i removed all the GlobalTranslationService stuff
as i18n gets injected in cgi.TranslationService - needs testing, classic
tracker works.

removed translationService from cgi.templating.py and cgi.form_parser.py

form_parser.py now uses cgi.TranslationService directly as noone else
needed the globally set translationService in templating.py

in wsgi_handler i removed the preload stuff and keep the tracker
instance over the requests - only the db gets opened again on every
request, templates and detectors only get computed once - as they cant 
change during running.

i removed the translate method in TALES.py as it gets injected as stated
above in cgi.TranslationService

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

caching would be nice, but as it just gets called once per request i
think we can go along with the current behaviour.

caching would also need to be thread safe ...

> 
>    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).

the problem is that i18n can change during processing of web requests,
therefore this does not work.

>>    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

done.

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

yeah, you are definitely right - my head hurts all the time ;)

> 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?

no, if they are not set, the NullTranslator - which gets set on db
instantiation, doesnt get changed.

for using i18n in the detectors you have to use db.i18n.gettext et al
like this:

def statusfail(db, cl, nodeid, newvalues):
     _ = db.i18n.gettext
     raise ValueError(_("this does not work"))

def init(db):
     # fire before changes are made
     db.status.audit('create', statusfail)

i have added a module "pygettext.py" which i am using in other projects
too which can extract msgid's from python code.

i have documented this a littte bit ;)

i extended roundup_gettext.py to use this module and to generate one
messages.pot file with both template strings and detector/extension strings.

this however depends on https://polib.readthedocs.io to merge the two
sources which i am not sure if you a) want a dependency in roundup, as i
have not seen any deps in setup.py or b) i have not found the correct
place to add it ... or c) just to document that it is a dependency if
you want to hack on your own tracker ...

attached is the patch against current tip, ralf will test it and modify
our tracker to use the db.i18n.gettext inside his detectors and then,
hopefully we have blazing fast rest API :)

... and hopefully it doesnt crash anything ...

regards,
marcus.
msg7470 Author: [hidden] (schlatterbeck) Date: 2022-04-28 08:05
Hi Marcus, thanks for the patch!

On Wed, Apr 27, 2022 at 03:50:43PM +0000, Marcus Priesch wrote:
[...]
> so here is what i have done:

[... i18n changes]

Preload-Stuff:
> in wsgi_handler i removed the preload stuff and keep the tracker
> instance over the requests - only the db gets opened again on every
> request, templates and detectors only get computed once - as they cant
> change during running.

.pot file creation:
> i have added a module "pygettext.py" which i am using in other projects
> too which can extract msgid's from python code.
> 
> i have documented this a littte bit ;)
> 
> i extended roundup_gettext.py to use this module and to generate one
> messages.pot file with both template strings and detector/extension strings.

> this however depends on https://polib.readthedocs.io to merge the two
> sources which i am not sure if you a) want a dependency in roundup, as i
> have not seen any deps in setup.py or b) i have not found the correct
> place to add it ... or c) just to document that it is a dependency if
> you want to hack on your own tracker ...

I'd like to keep the Preload and .pot-creation changes separate (as
separate commits) from the i18n changes. The preload changes have high
potential for breakage, I remember to do changes in that part to make my
reverse multilink feature work (which relies on a callback that never
came under certain circumstances). Also not preloading the detectors and
the actions is nice for debugging but I'm all in favour of making the
code simpler in that area at the cost of having to restart services when
debugging.

Also note that the .pot-creation changes probably fixes a longstanding
problem that for creating the .pot files an outdated unmaintained
package 'xpot' is currently used, see locale/GNUmakefile, the package is
hard to build (I only succeeded in building the xpot binary, not the
rest of the package) and is written in C (for parsing python)

I'll keep you posted...

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg7472 Author: [hidden] (marcus.priesch) Date: 2022-04-28 08:37
BIG SORRY !

the patch is missing one self. in wsgi_handler.py.

please find attached the correct one !

ps: do you have anything i can test against the preloading stuff?

regards,
marcus.
msg7473 Author: [hidden] (schlatterbeck) Date: 2022-04-28 09:38
On Thu, Apr 28, 2022 at 08:37:14AM +0000, Marcus Priesch wrote:

> ps: do you have anything i can test against the preloading stuff?

Hmm, not really. The original issue was issue2551172 where the rev
multilinks were only there on the first request (with the standalone
roundup_server) but not on subsequent requests when the connection was
cached. The problem did *not* materialize with WSGI. And the first
request after the cached connection timed out was OK again.

So I really don't know how to come up with an easy test for this.
Especially when WSGI is involved (although *this* problem didn't occur
with WSGI another one might be specific to WSGI).

But maybe your patches make the code *less* brittle in this part so that
the tracker (as opposed to the DB) is only always initialized once on
startup -- for WSGI or other frontends.

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg7474 Author: [hidden] (rouilj) Date: 2022-04-28 12:48
Hi all:

In message <20220428093811.l6y4srn5qt4bzxcw@runtux.com>,
Ralf Schlatterbeck writes:
>Ralf Schlatterbeck added the comment:
>On Thu, Apr 28, 2022 at 08:37:14AM +0000, Marcus Priesch wrote:
>> ps: do you have anything i can test against the preloading stuff?
>
>Hmm, not really. The original issue was issue2551172 where the rev
>multilinks were only there on the first request (with the standalone
>roundup_server) but not on subsequent requests when the connection was
>cached. The problem did *not* materialize with WSGI. And the first
>request after the cached connection timed out was OK again.
>
>So I really don't know how to come up with an easy test for this.
>Especially when WSGI is involved (although *this* problem didn't occur
>with WSGI another one might be specific to WSGI).

We do have a live_server test using the wsgi backend in the test
bed. Not sure if it's useful for this problem.

>But maybe your patches make the code *less* brittle in this part so that
>the tracker (as opposed to the DB) is only always initialized once on
>startup -- for WSGI or other frontends.

Roundup-server is supposed to have its own debug mode that reloads
templates if they have changed. I thought there was a debug mode that
made the same thing happen in the other front end deployment
mechanisms. I rely on this extensvely for tracker development. Not
having hot reload can mean literally hundreds of server restarts as
template changes are done.
msg7475 Author: [hidden] (marcus.priesch) Date: 2022-04-28 15:31
> Roundup-server is supposed to have its own debug mode that reloads
> templates if they have changed. I thought there was a debug mode that
> made the same thing happen in the other front end deployment
> mechanisms. I rely on this extensvely for tracker development. Not
> having hot reload can mean literally hundreds of server restarts as
> template changes are done.

i am not aware of this - how can i enable this to test if i18n breaks ?

but then, you would also like to have hot-reload when .mo files change,
which is a different story then ...

regards,
m.
msg7476 Author: [hidden] (marcus.priesch) Date: 2022-04-28 15:34
here is the current patch.

i reverted some of the too optimistic deletions and fixed i18n issues in
the error handling case.

it works under wsgi, cli and roundup-server

regards,
marcus.
msg7477 Author: [hidden] (schlatterbeck) Date: 2022-04-28 17:00
On Thu, Apr 28, 2022 at 03:34:50PM +0000, Marcus Priesch wrote:
> 
> Marcus Priesch added the comment:
> 
> here is the current patch.
> 
> i reverted some of the too optimistic deletions and fixed i18n issues in
> the error handling case.
> 
> it works under wsgi, cli and roundup-server

For the previous version of the patch I'm getting some errors during testing
of the anydbm backend:

>       self.translator = client.translator
E       AttributeError: Client instance has no attribute 'translator'

roundup/rest.py:442: AttributeError

In roundup/rest.py line 442 the code tries to access client.translator
which doesn't seem to exist.

You should try to run the tests:

python -m pytest test

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg7478 Author: [hidden] (rouilj) Date: 2022-04-28 18:24
Hi Marcus:

In message <6bb34266-f3cf-96d2-60c7-09d2b4c50513@priesch.co.at>,
Marcus Priesch writes:
>> Roundup-server is supposed to have its own debug mode that reloads
>> templates if they have changed. I thought there was a debug mode that
>> made the same thing happen in the other front end deployment
>> mechanisms. I rely on this extensvely for tracker development. Not
>> having hot reload can mean literally hundreds of server restarts as
>> template changes are done.
>
>i am not aware of this - how can i enable this to test if i18n breaks ?

I think it's 

   roundup-server -t debug

the comment in scripts/roundup_server.py is:

   # "debug" means "none" + no tracker/template cache

which I think makes it single threaded as well.

>but then, you would also like to have hot-reload when .mo files change,
>which is a different story then ...

Wouldn't .mo files be loaded on every call to determine_language() and
reset on every database open which happens on each request?
msg7480 Author: [hidden] (marcus.priesch) Date: 2022-04-29 10:11
Hello Ralf,

> python -m pytest test

thanks a lot, would be nice if that would have found it's way into
developers.txt - with all the other stuff one needs to run the tests
correctly. maybe we could make a docker container where everything is
set up correctly to run the tests ?

here is my next patch where only one test in test_liveserver.py fails.

the interesting part is that when i only run the test alone it does not
fail ?

$ LANG="" python -m pytest test/test_liveserver.py
========================== test session starts ===========================
platform linux2 -- Python 2.7.18, pytest-4.6.11, py-1.11.0, pluggy-0.13.1
rootdir: /home/pr/work/projects/5t-roundup/roundup.hg
collected 20 items

test/test_liveserver.py X...................                       [100%]

================= 19 passed, 1 xpassed in 10.23 seconds ==================

when i run all tests, it fails:

test/test_liveserver.py x.................F.  [ 34%]

hmmm ...

i also fixed test_dates.py which did not find the locales, i am not sure
if i do the test setup in the correct way - but as i did not change
anything in roundup.i18n ... did this test work at all ?

regards,
marcus.
msg7490 Author: [hidden] (schlatterbeck) Date: 2022-05-03 13:13
I've now pushed the latest version of the patch with the following exceptions:
- The patches for building templates (.pot etc)
- The patch to cgi/wsgi_handler.py: I'm still not sure what it improves
I had tested the whole thing *with* the patches above (without any tests failing, the failing tests reported by Marcus are fixed in a separate commit). I'm now in the process of testing without the two exceptions above.

I'll clarify with Marcus what to to with cgi/wsgi_handler.py.

We'll also come up with modifications to how templates are built. The file Marcus provided as pygettext.py is part of the standard python distribution on debian (and is called pygettext3 for python3). Seems the file is part of the standard python source distribution but is not shipped by all Linux distros. And it will not help much when the file is available as a script: The patches to scripts/roundup_gettext.py would need to import it (and not use it as a script). The version in Debian is newer and is already ported to python3.
msg7499 Author: [hidden] (rouilj) Date: 2022-05-03 18:43
Pushed changes to CI. Code coverage is at:

 https://codecov.io/gh/roundup-tracker/roundup/commit/32468930a3d9df4534356b3eeffc0ee7ce08f37e/

It looks like we have no coverage of the changes in mailgw.py. Any idea how to test that?

Also it looks like the rest and xmlrpc tests always run with translations off.
Does adding a test case that enables translation ans chooses the german locale
make sense. I figure pull the status info and verify that the status
names are translated.
msg7503 Author: [hidden] (marcus.priesch) Date: 2022-05-04 06:46
> Also it looks like the rest and xmlrpc tests always run with 
> translations off. Does adding a test case that enables translation 
> ans chooses the german locale make sense. I figure pull the status 
> info and verify that the status names are translated.

this definitely would make sense, however, the default in config.ini is
to not translate rest and xmlrpc - so we need to change that before the
test. which needs to go into db_test_base.py:setupTracker ? or can this
be done at a later stage ... sorry, i am not that deep in the testing stuff.

for mailgw i dont know if we would need to setup a detector that creates
some i18n'd message ? or should it also send translated issue status ?

regards,
marcus.
msg7505 Author: [hidden] (rouilj) Date: 2022-05-04 15:22
Hi Marcus:

> the default in config.ini is to not translate rest and xmlrpc - so
> we need to change that before the test.

Correct. If you look at test/rest_common.py you can see code like:

   self.db.config['WEB_JWT_SECRET'] = secret

self.db.config['...'] is the config object used by the test. So in
this case we would set:

   self.db.config['WEB_TRANSLATE_REST"] = 'yes'
   
or
   self.db.config['WEB_TRANSLATE_XMLRPC'] = 'yes'
      
before calling the code that does the request. This can be changed on
the fly in tests not using the live server (test_liveserver.py). I
usually store the original value, change it and restore at the end of
the test. In most cases, each test generates a fresh db.config but I
have been bit by my config changes leaking into other tests.

I think the translation for rest can be verified using a test in
rest_common.py by setting the right config option and setting the
proper headers in self.headers in rest_common.py.

(For the live server we have to shut down and start the server with
the new config which can probably be done but.... Probably better to
have multiple live_serverN.py files that set the parameters we want in
SimpleTest::setup_class and just use the server for all the tests.)

> db_test_base.py:setupTracker ? or can this be done at a later stage
> ... sorry, i am not that deep in the testing stuff.

It can be done at the test level, not setup. I am not great at the
testing stuff either.  However we just reached 75% code coverage and I
don't want to slide back 8-).

> for mailgw i dont know if we would need to setup a detector that
> creates some i18n'd message ? or should it also send translated
> issue status ?

That's a good question. I looked for _( in mailgw.py to see what
strings are marked to be translated. It looks like:

  Emails to Roundup trackers must include a Subject: line!

is one such string. Sadly de.po doesn't have a transation for that
string 8-(. So I guess:

 1) translate that string for german and generate a .mo from
    locale/de.po
 2) craft a test case from existing tests n test_mailgw.py that
    is missing the Subject. Verify the error shows up in the result
    using self._get_mail().
 3) set self.db.config['MAILGW_LANGUAGE'] = 'de' and verify new
    translation shows up.

that should provide basic coverage.

For extra credit:

  test with self.db.config['MAILGW_LANGUAGE'] = "" and
    self.db.config['TRACKER_LANGUAGE'] = 'de'
    
(IIRC you fixed the code for this test to work right. Thanks.)
msg7507 Author: [hidden] (rouilj) Date: 2022-05-04 22:28
Ok testing mailgw.py is getting weird.

While setting the db.config vars as I mentioned should work, there is a glitch.
The handle_message method which sets the language by looking at the db.config vars
is never called by the test harness. The test routine calls _handle_message (which is
called by handle_message) directly.

I think this is because handle_message() opens and closes the database. This would wipe
out the db and config setting that are crafted for the tests.

So I extracted code from handle_message that sets up the language translation services.
This would set the test's db.i18n property and the global _ variable.
I already imported i18n in the test.

In this code (mailgw.py:1606):

        language = self.instance.config["MAILGW_LANGUAGE"] or 
self.instance.config["TRACKER_LANGUAGE"]
        self.db.i18n = i18n.get_language (language)

is what I copied. However i18n.get_language() isn't a valid method.
I am not sure what i18n refers to. The mailgw.py doesn't import i18n anywhere.

Should get_language() be get_translation() (which does exist in i18n)?

Ideas? If a patch is added for this problem please name it "mailgw_lang" or something
to differentiate it from the other patches. It should probably get a new issue actually
but....
msg7508 Author: [hidden] (marcus.priesch) Date: 2022-05-05 09:11
> In this code (mailgw.py:1606):
> 
>          language = self.instance.config["MAILGW_LANGUAGE"] or
> self.instance.config["TRACKER_LANGUAGE"]
>          self.db.i18n = i18n.get_language (language)
> 
> is what I copied. However i18n.get_language() isn't a valid method.
> I am not sure what i18n refers to. The mailgw.py doesn't import i18n anywhere.
> 
> Should get_language() be get_translation() (which does exist in i18n)?

uh, oh, that is not good - this was my fault - never tested the mailgw !

indeed the import is missing:

	from roundup import i18n

and indeed you need to call:

	self.db.i18n = i18n.get_translation (language)

so this is the patch for current tip:

diff -r ac0df9272162 roundup/mailgw.py
--- a/roundup/mailgw.py Tue May 03 13:16:28 2022 -0400
+++ b/roundup/mailgw.py Thu May 05 11:09:45 2022 +0200
@@ -109,6 +109,7 @@
  from roundup import configuration, hyperdb, date, password, exceptions
  from roundup.mailer import Mailer
  from roundup.i18n import _
+from roundup import i18n
  from roundup.hyperdb import iter_roles
  from roundup.anypy.strings import StringIO, b2s, u2s
  import roundup.anypy.random_ as random_
@@ -1604,7 +1605,7 @@
          self.db = self.instance.open('admin')

          language = self.instance.config["MAILGW_LANGUAGE"] or 
self.instance.config["TRACKER_LANGUAGE"]
-        self.db.i18n = i18n.get_language (language)
+        self.db.i18n = i18n.get_translation (language)

          global _
          _ = self.db.i18n.gettext

sorry,
marcus.
msg7514 Author: [hidden] (rouilj) Date: 2022-05-12 02:47
Ok, got the test for mailgw.py fixed. Had to manually assign the new translator to
roundupdb._ and mailgw._. I couldn't just assign it to a global _.

But be that as it may.

This leaves the .po generation (docs and code) and wsgi changes as described in msg7490.
Also we need testing for translations with rest and xmlrpc.

Do we have anything else pending on this issue?
msg7783 Author: [hidden] (rouilj) Date: 2023-05-30 19:23
I think the wsgi changes have been done and were available behind a feature flag
msg7807 Author: [hidden] (rouilj) Date: 2023-07-21 00:33
I took another look at this.

The patch for test_dates.py has been applied and committed.

I opened issue2551287 for the detector and extensions translation extraction
for roundup_gettext.

and I declare this complete. Thanks Marcus.
History
Date User Action Args
2023-07-21 00:33:49rouiljsetstatus: open -> fixed
resolution: remind -> fixed
messages: + msg7807
2023-05-30 19:23:07rouiljsetresolution: remind
messages: + msg7783
2022-05-12 02:47:46rouiljsetmessages: + msg7514
2022-05-05 09:11:02marcus.prieschsetmessages: + msg7508
2022-05-04 22:28:56rouiljsetmessages: + msg7507
2022-05-04 15:22:47rouiljsetmessages: + msg7505
2022-05-04 06:46:00marcus.prieschsetmessages: + msg7503
2022-05-03 18:43:51rouiljsetmessages: + msg7499
2022-05-03 17:19:42rouiljsetpriority: normal
assignee: schlatterbeck
status: new -> open
2022-05-03 13:13:25schlatterbecksetmessages: + msg7490
2022-04-29 10:11:03marcus.prieschsetfiles: + improve_i18n_4.patch
messages: + msg7480
2022-04-28 18:24:34rouiljsetmessages: + msg7478
2022-04-28 17:00:51schlatterbecksetmessages: + msg7477
2022-04-28 15:34:50marcus.prieschsetfiles: + improve_i18n_3.patch
messages: + msg7476
2022-04-28 15:31:18marcus.prieschsetmessages: + msg7475
2022-04-28 12:48:49rouiljsetmessages: + msg7474
2022-04-28 09:38:14schlatterbecksetmessages: + msg7473
2022-04-28 08:37:14marcus.prieschsetfiles: + improve_i18n_2.patch
messages: + msg7472
2022-04-28 08:05:30schlatterbecksetmessages: + msg7470
2022-04-27 15:50:43marcus.prieschsetfiles: + improve_i18n.patch
keywords: + patch
messages: + msg7469
2022-03-22 08:21:03marcus.prieschsetmessages: + msg7462
2022-03-06 03:19:04rouiljsetmessages: + msg7459
2021-12-17 00:41:26rouiljsetnosy: + rouilj
messages: + msg7424
2021-12-16 13:37:21marcus.prieschcreate