]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Remove gettext.install() from cinder/__init__.py
authorMark McLoughlin <markmc@redhat.com>
Wed, 8 May 2013 12:52:42 +0000 (13:52 +0100)
committerZhiteng Huang <zhiteng.huang@intel.com>
Mon, 13 May 2013 10:45:25 +0000 (18:45 +0800)
The gettext.install() function installs a builtin _() function which
translates a string in the translation domain supplied to the install()
function. If gettext.install() is called multiple times, it's the last
call to the function which wins and the last supplied translation domain
which is used e.g.

 >>> import os
 >>> os.environ['LANG'] = 'ja.UTF-8'
 >>> import gettext
 >>> gettext.install('keystone', unicode=1, localedir='/opt/stack/keystone/keystone/locale')
 >>> print _('Invalid syslog facility')
  n無効な syslog ファシリティ
 >>> gettext.install('cinder', unicode=1, localedir='/opt/stack/nova/cinder/locale')
 >>> print _('Invalid syslog facility')
 Invalid syslog facility

Usually this function is called early on in a toplevel script and we
assume that no other code will call it and override the installed _().
However, in Cinder, we have taken a shortcut to avoid having to call it
explicitly from each script and instead call it from cinder/__init__.py.

This shortcut would be perfectly fine if we were absolutely sure that
nova modules would never be imported from another program. It's probably
quite incorrect for a program to use cinder code (indeed, if we wanted
to support this, Cinder code shouldn't use the default _() function) but
nevertheless there are some corner cases where it happens. For example,
the keystoneclient auth_token middleware tries to import cfg from
cinder.openstack and this in turn causes gettext.install('cinder') in
other projects like glance or quantum.

To avoid any doubt here, let's just rip out the shortcut and always
call gettext.install() from the top-level script.

However, there's a bit of an annoying detail here - by default,
nosetests starts in the current directly and tries to import all modules
it finds to look for tests. Without the _() builtin installed, importing
some modules like cinder.flags will fail.

Since it only ever makes sense to load tests from the cinder/tests dir,
we can ask nose to do that by using the --tests argument via setup.cfg.

Note, this means that if you previously did this:

  $> tox -- cinder.tests.foo cinder.tests.bar

then you must now do this:

  $> tox -- --tests cinder.tests.foo,cinder.tests.bar

Change-Id: If4125d6bcbde63df95de129ac5c83b4a6d6f130a

bin/cinder-all
bin/cinder-api
bin/cinder-backup
bin/cinder-volume
cinder/__init__.py
doc/source/devref/il8n.rst
setup.cfg

index e384e89c6ab0771aef49eab84e10cde04051ad4e..6cfeedcce9bcd5b599df96c7c15b864087eeb0e7 100755 (executable)
@@ -30,6 +30,7 @@ continue attempting to launch the rest of the services.
 import eventlet
 eventlet.monkey_patch()
 
+import gettext
 import os
 import sys
 
@@ -39,6 +40,7 @@ possible_topdir = os.path.normpath(os.path.join(os.path.abspath(
 if os.path.exists(os.path.join(possible_topdir, "cinder", "__init__.py")):
     sys.path.insert(0, possible_topdir)
 
+gettext.install('cinder', unicode=1)
 
 from cinder import flags
 from cinder.openstack.common import log as logging
index 13c3db5112ec8a2fa2c7c37f7ec5286fa7fb4034..68c06782b83ccb4e51dc49c1ec35f1fe52c2f792 100755 (executable)
@@ -26,6 +26,7 @@
 import eventlet
 eventlet.monkey_patch()
 
+import gettext
 import os
 import sys
 
@@ -35,6 +36,7 @@ possible_topdir = os.path.normpath(os.path.join(os.path.abspath(
 if os.path.exists(os.path.join(possible_topdir, "cinder", "__init__.py")):
     sys.path.insert(0, possible_topdir)
 
+gettext.install('cinder', unicode=1)
 
 from cinder import flags
 from cinder.openstack.common import log as logging
index 0ffadec9beadf529acf32745cfe2324b068721e4..d0278a35ad573a2546b5b13b227217d0d2735c14 100755 (executable)
@@ -17,6 +17,7 @@
 
 """Starter script for Cinder Volume Backup."""
 
+import gettext
 import os
 import sys
 
@@ -32,6 +33,7 @@ possible_topdir = os.path.normpath(os.path.join(os.path.abspath(sys.argv[0]),
 if os.path.exists(os.path.join(possible_topdir, 'cinder', '__init__.py')):
     sys.path.insert(0, possible_topdir)
 
+gettext.install('cinder', unicode=1)
 
 from cinder import flags
 from cinder.openstack.common import log as logging
index 90f6b1c0597fffb9d83490dd3ba09ce46f209607..bf144830b261ee05f4fe2b861ff5312c0934e7b4 100755 (executable)
@@ -22,6 +22,7 @@
 import eventlet
 eventlet.monkey_patch()
 
+import gettext
 import os
 import sys
 
@@ -33,6 +34,7 @@ possible_topdir = os.path.normpath(os.path.join(os.path.abspath(sys.argv[0]),
 if os.path.exists(os.path.join(possible_topdir, 'cinder', '__init__.py')):
     sys.path.insert(0, possible_topdir)
 
+gettext.install('cinder', unicode=1)
 
 from cinder import flags
 from cinder.openstack.common import log as logging
index f8db8e875cf2446fe1ab03717fd2aa58d1e9ea2b..d765b088e93f62b9409a59942cbd91a2ae43d505 100644 (file)
@@ -30,8 +30,3 @@
 .. moduleauthor:: Manish Singh <yosh@gimp.org>
 .. moduleauthor:: Andy Smith <andy@anarkystic.com>
 """
-
-import gettext
-
-
-gettext.install('cinder', unicode=1)
index fabd2ce49f465d3eeedecbcb8cfae5e75717339b..7e53731d743083c27aaa8220bff2dcf30bade548 100644 (file)
@@ -27,8 +27,7 @@ The ``_()`` function is brought into the global scope by doing::
     import gettext
     gettext.install("cinder", unicode=1)
 
-In general, you shouldn't need to add these to any cinder files, since the lines
-are present in ``cinder/__init__.py``. However, if this code is missing, it may
-result in an error that looks like like::
+These lines are needed in any toplevel script before any cinder modules are
+imported. If this code is missing, it may result in an error that looks like::
 
     NameError: name '_' is not defined
index 3f72e16c7877ab7292029b0573a78fc9b67f4da8..37e5093caf04554212c4c77f46173af28feddf55 100644 (file)
--- a/setup.cfg
+++ b/setup.cfg
@@ -23,6 +23,7 @@ mapping_file = babel.cfg
 output_file = cinder/locale/cinder.pot
 
 [nosetests]
+tests=cinder/tests
 cover-package = cinder
 cover-erase = true
 cover-inclusive = true