Re: [Freeipa-devel] [PATCH] Send Accept-Language header over XML-RPC and translate on server.

2011-02-16 Thread Pavel Zůna

On 2011-02-04 18:35, Pavel Zůna wrote:

On 2011-02-04 16:23, Rob Crittenden wrote:

Pavel Zuna wrote:

This patch makes the ipa client send the Accept-Language header, so that
the server can translate things like exceptions, that cannot be
translated on the client.

It also fixes the language recognition for the webUI. The values in
Accept-Language header are a bit different than what is accepted by the
LANG variable as a valid locale - some additional parsing was needed.
For example:
 Accept-Language: es-es;q=1
needs to translate to
 es_ES
otherwise it won't be recognized by gettext

Fix #904
Fix #917

Pavel


nack.

ast is imported but not used


Leftover. Removed in the attached updated version.


Why are you calling locale.setlocale() instead of locale.getlocale()?


Because that's how it should be done. setlocale() with an empty string
as second argument gets the current environment settings. getlocale()
without a previous call to setlocale returns (None, None).


If extra_headers is passed in as a string this will drop it:


That's never going to happen. I checked the underlying implementation in
xmlrpclib and it can either be a list or dict. In this case,
LanguageAwareTransport is calling Transport.get_host_info() which always
returns extra_headers as a list or None if empty.

The original implementation (before this patch) always dropped the whole
thing and used a new list instead.


+ if not isinstance(extra_headers, list):
+ extra_headers = []

Multiple Authorization is actually legal though it may be a good idea to
remove any others found, so I'll let this part go. I don't know that it
is really needed though.


Because the underlying Transport class can fill Authorization with
'Basic auth' and the original implementation was dropping it as well.


Some formatting is changed to make it less readable IMHO:

- else:
- scheme = http
+ else: scheme = http


That's unintentional, sorry.


The code to break HTTP_ACCEPT_LANGUAGE into language and region is
broken. Passing in en-gb returns en_EN. (I think you want [1] not [0]).


Nice catch. I was probably thinking that since I'm using rsplit(), the
indexes will be the other way around. :) Fixed in attached version.


Ideally we would loop through all acceptable languages until we find one
that we actually provide.

So if we are passed in da, en-gb;q=0.8, en;q=0.7 we would first look for
Danish but fall back to British English or any other English (preferring
British English).


That's a good idea! However I would keep it simple for now and do this
in a separate patch.


rob


Pavel




Rebased version attached.

Pavel


freeipa-pzuna-71-3-acceptlang.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] Send Accept-Language header over XML-RPC and translate on server.

2011-02-04 Thread Pavel Zuna
This patch makes the ipa client send the Accept-Language header, so that the 
server can translate things like exceptions, that cannot be translated on the 
client.


It also fixes the language recognition for the webUI. The values in 
Accept-Language header are a bit different than what is accepted by the LANG 
variable as a valid locale - some additional parsing was needed.

For example:
 Accept-Language: es-es;q=1
needs to translate to
 es_ES
otherwise it won't be recognized by gettext

Fix #904
Fix #917

Pavel


freeipa-pzuna-71-acceptlang.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] Send Accept-Language header over XML-RPC and translate on server.

2011-02-04 Thread Rob Crittenden

Pavel Zuna wrote:

This patch makes the ipa client send the Accept-Language header, so that
the server can translate things like exceptions, that cannot be
translated on the client.

It also fixes the language recognition for the webUI. The values in
Accept-Language header are a bit different than what is accepted by the
LANG variable as a valid locale - some additional parsing was needed.
For example:
  Accept-Language: es-es;q=1
needs to translate to
  es_ES
otherwise it won't be recognized by gettext

Fix #904
Fix #917

Pavel


nack.

ast is imported but not used

Why are you calling locale.setlocale() instead of locale.getlocale()?

If extra_headers is passed in as a string this will drop it:

+if not isinstance(extra_headers, list):
+extra_headers = []

Multiple Authorization is actually legal though it may be a good idea to 
remove any others found, so I'll let this part go. I don't know that it 
is really needed though.


Some formatting is changed to make it less readable IMHO:

-else:
-scheme = http
+else: scheme = http

The code to break HTTP_ACCEPT_LANGUAGE into language and region is 
broken. Passing in en-gb returns en_EN. (I think you want [1] not [0]).


Ideally we would loop through all acceptable languages until we find one 
that we actually provide.


So if we are passed in da, en-gb;q=0.8, en;q=0.7 we would first look for 
Danish but fall back to British English or any other English (preferring 
British English).


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel