Issue 2551184
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:49 | rouilj | set | status: open -> fixed resolution: remind -> fixed messages: + msg7807 |
2023-05-30 19:23:07 | rouilj | set | resolution: remind messages: + msg7783 |
2022-05-12 02:47:46 | rouilj | set | messages: + msg7514 |
2022-05-05 09:11:02 | marcus.priesch | set | messages: + msg7508 |
2022-05-04 22:28:56 | rouilj | set | messages: + msg7507 |
2022-05-04 15:22:47 | rouilj | set | messages: + msg7505 |
2022-05-04 06:46:00 | marcus.priesch | set | messages: + msg7503 |
2022-05-03 18:43:51 | rouilj | set | messages: + msg7499 |
2022-05-03 17:19:42 | rouilj | set | priority: normal assignee: schlatterbeck status: new -> open |
2022-05-03 13:13:25 | schlatterbeck | set | messages: + msg7490 |
2022-04-29 10:11:03 | marcus.priesch | set | files:
+ improve_i18n_4.patch messages: + msg7480 |
2022-04-28 18:24:34 | rouilj | set | messages: + msg7478 |
2022-04-28 17:00:51 | schlatterbeck | set | messages: + msg7477 |
2022-04-28 15:34:50 | marcus.priesch | set | files:
+ improve_i18n_3.patch messages: + msg7476 |
2022-04-28 15:31:18 | marcus.priesch | set | messages: + msg7475 |
2022-04-28 12:48:49 | rouilj | set | messages: + msg7474 |
2022-04-28 09:38:14 | schlatterbeck | set | messages: + msg7473 |
2022-04-28 08:37:14 | marcus.priesch | set | files:
+ improve_i18n_2.patch messages: + msg7472 |
2022-04-28 08:05:30 | schlatterbeck | set | messages: + msg7470 |
2022-04-27 15:50:43 | marcus.priesch | set | files:
+ improve_i18n.patch keywords: + patch messages: + msg7469 |
2022-03-22 08:21:03 | marcus.priesch | set | messages: + msg7462 |
2022-03-06 03:19:04 | rouilj | set | messages: + msg7459 |
2021-12-17 00:41:26 | rouilj | set | nosy:
+ rouilj messages: + msg7424 |
2021-12-16 13:37:21 | marcus.priesch | create |