Apache Mod-Python - Graham: Suggested change to importer.py

Author

Graham: Suggested change to importer.py

Dan Eloff

2006-08-12, 1:12 pm

I had some modules with circular imports and I only discovered that
this was a problem when I tried to discover why my modules were being
reimported constantly. I would suggest that this code be changed to
log an error that explicitly tells you that there's a circular import,
and what the consequences are.

# Check for a child which refers to one of its
# ancestors. Hopefully this will never occur. If
# it does we will force a reload every time to
# highlight there is a problem.

if label in ancestors:
# LOG HELPFUL ERROR HERE
return True

Also, I still chafe every time I have to write:

sidewinder_dir = r'C:\Docume~1\Dan\MyDocu~1\PYROOT\sidewi
nder\'
from mod_python import apache
session = apache.import_module(sidewinder_dir+'session.py')
config = apache.import_module(sidewinder_dir+'config.py')

Instead of:
import session
import config

My handler imports modules like so:

IMPORT_PATH = [config.server_options['sidewinder_dir'], config.web['root']]
apache.import_module(mpath, path=IMPORT_PATH)

Which works great, but when those modules import something I have to
use the ugly mess above. Maybe you and I could think of a better way?

I'd like if I could just add an option globally to my httpd.conf like:
PythonOption ChildrenAlwaysInheritImportPath On

Or maybe add another parameter to import_module like:
apache.import_module(mpath, path=IMPORT_PATH, inherit=True)

Or?

Thanks,
-Dan

Graham Dumpleton

2006-08-13, 1:12 am

On 13/08/2006, at 3:43 AM, Dan Eloff wrote:

> I had some modules with circular imports and I only discovered that
> this was a problem when I tried to discover why my modules were being
> reimported constantly. I would suggest that this code be changed to
> log an error that explicitly tells you that there's a circular import,
> and what the consequences are.
>
> # Check for a child which refers to one of its
> # ancestors. Hopefully this will never occur. If
> # it does we will force a reload every time to
> # highlight there is a problem.
>
> if label in ancestors:
> # LOG HELPFUL ERROR HERE
> return True

That looks reasonable. Should the warning message always be displayed in
the Apache error log, even if PythonDebug is Off?

> Also, I still chafe every time I have to write:
>
> sidewinder_dir = r'C:\Docume~1\Dan\MyDocu~1\PYROOT\sidewi
nder\'
> from mod_python import apache
> session = apache.import_module(sidewinder_dir+'session.py')
> config = apache.import_module(sidewinder_dir+'config.py')
>
> Instead of:
> import session
> import config

One can clean it up a bit and have:

__mp_path__ += & #91;r'C:\Docume~1\Dan\MyDocu~1\PYROOT\si
dewinder\']

from mod_python import apache
import session
import config

Or even:

from mod_python import apache

MAIN_SERVER_CONFIG = apache.main_server.get_config()
CONFIG_MODULE_PATH = MAIN_SERVER_CONFIG['sidewinder_config']
__mp_path__ += [CONFIG_MODULE_PATH]

import session
import config

The PythonOption has to be part of the main Apache configuration
though and
not part of the VirtualHost or per request configuration, but at
least the path isn't
embedded in the module.

Even so, do appreciate that it isn't really ideal. But then, I do
find it a bit strange
what you are doing in that you seem to be having a generic module
know where
to find something specific to an application it is being used by, by
virtue of what
is in module search path, rather than the application making use of
the generic
module supplying the configuration modules as arguments to functions
in generic
module that may be called by actual application.

Anyway, I have come to the conclusion that one can't avoid having an
equivalent to
the PythonPath directive but which defines directories to be searched
by the new
importer. If people use this in the wrong way and hang themselves
then that is
their problem. In general they would be okay though as long as it is
defined once
only at the root corresponding to a specific Python interpreter
instance.

What I am not sure about yet is whether the means of setting the path
should
be done with a new directive or using PythonOption. As I mentioned in
mail
just sent, one reason for using a new directive, is that if
PythonAllowOverride
is introduced, configuration in main Apache configuration could set
the module
search path and at the same time prohibit the user changing the
default. This
would be needed where for example the user was also prohibited from
defining
their own authentication handlers and there was a global one that used
mod_python handlers. The user could override the default and screw up
the operation of the globally defined authentication handlers if
those handlers
were expecting to find a module by searching that path but by user
changing
it, they could no longer be found. But then, in this situation it may
be better for
the authentication handler to load the module by an explicit path
thereby avoiding
the problem altogether.

Now by using a directive though, it would make it harder for a user
handler,
for example one that ran in the fixup phase, to dynamically modify
the search
path for modules for all response handlers executed within a certain
context.
Specifically, if the search path is set by a PythonOption, then the
fixup handler
could actually modify the inherited value of the option by modifying
the value
stored in the req.get_options() table object. If the proposed
PythonAllowOverride
directive can disallow setting of specific options using
PythonOption, then a
good middle ground might be found as one could block the options
being set
in a .htaccess file and therefore it would not interfere with a
global authentication
handler, but a fixup handler could be allowed to still set it for the
subsequent
response handler. But then, I am talking hypotheticals here and maybe
I am
just thinking too hard. Thinking up such scenarios though, is what is
required
in making good generic reusable modules or frameworks. :-)

> My handler imports modules like so:
>
> IMPORT_PATH = [config.server_options['sidewinder_dir'], config.web
> ['root']]
> apache.import_module(mpath, path=IMPORT_PATH)
>
> Which works great, but when those modules import something I have to
> use the ugly mess above.

Is IMPORT_PATH different to or the same as:

r'C:\Docume~1\Dan\MyDocu~1\PYROOT\sidewi
nder\'

Anyway, the other way of doing the above such that a search directory is
inherited, is to rather than use a module name, translate that to a
full path and
import that. The 'path' argument to apache.import_module() can then
be set
to be a path embedded as __mp_path__ in the module loaded. Ie. something
like:

GENERIC_MODULE_PATH = '/some/path/generic'
CONFIG_MODULE_PATH = '/some/path/config'

GENRIC_MODULE_FILE = os.path.join(GENERIC_MODULE_PATH, "module.py")

module = apache.import_module(GENERIC_MODULE_FILE, path=
[CONFIG_MODULE_PATH])

The path though is only inherited by the module immediately imported
though
and not further.

Originally I probably steered you away from this, because if that
generic module
is imported from multiple locations and not all consistently set the
path to the same,
whatever one got executed first would have their path be used, which
may not be
correct for all use cases. Thus, why I suggested that the generic
module itself dictate
itself any additional path by setting __mp_path__ internally to the
module when it
is being imported.

> Maybe you and I could think of a better way?

We can certainly explore other options, just wary of making it too
easy to introduce
non deterministic behaviour like exists now.

> I'd like if I could just add an option globally to my httpd.conf like:
> PythonOption ChildrenAlwaysInheritImportPath On

This would make things a bit unpredictable again.

> Or maybe add another parameter to import_module like:
> apache.import_module(mpath, path=IMPORT_PATH, inherit=True)

Which is what path option does when mpath is an absolute path. It
just doesn't
keep being inherited unless module imported does the same thing.

> Or?

Simpler just to allow a global search path like PythonPath that is
embedded into
modules when they are imported. It would be embedded so that its
value can't
change over time for a module that has already been imported.

Graham

Dan Eloff

2006-08-13, 7:21 pm

On 8/12/06, Graham Dumpleton <grahamd@dscpl.com.au> wrote:
>
> On 13/08/2006, at 3:43 AM, Dan Eloff wrote:
>
[vbcol=seagreen]
>
> That looks reasonable. Should the warning message always be displayed in
> the Apache error log, even if PythonDebug is Off?

Tough call. With PythonDebug Off you get a million messages logged
saying that the module is being reimported and cloned etc anyway
(correct me if that's mistaken) so logging another message probably
doesn't matter. I'd recommend not logging any of that if PythonDebug
is Off.

<snip>

> Even so, do appreciate that it isn't really ideal. But then, I do
> find it a bit strange
> what you are doing in that you seem to be having a generic module
> know where
> to find something specific to an application it is being used by, by
> virtue of what
> is in module search path, rather than the application making use of
> the generic
> module supplying the configuration modules as arguments to functions
> in generic
> module that may be called by actual application.

Yes that would be a little odd. This is not a generic module, it's
very application specific and holds misc constants and functions that
are used in more than one module. It's quite meaningless outside the
scope of the application, and so you can probably see why I want it to
be able to import application modules. I usually put generic modules
on sys.path, and I wouldn't want any of them having any knowledge of a
specific applciation.

>
> Anyway, I have come to the conclusion that one can't avoid having an
> equivalent to
> the PythonPath directive but which defines directories to be searched
> by the new
> importer. If people use this in the wrong way and hang themselves
> then that is
> their problem. In general they would be okay though as long as it is
> defined once
> only at the root corresponding to a specific Python interpreter
> instance.

> What I am not sure about yet is whether the means of setting the path
> should
> be done with a new directive or using PythonOption. As I mentioned in
> mail
> just sent, one reason for using a new directive, is that if
> PythonAllowOverride
> is introduced, configuration in main Apache configuration could set
> the module
> search path and at the same time prohibit the user changing the
> default. This
> would be needed where for example the user was also prohibited from
> defining
> their own authentication handlers and there was a global one that used
> mod_python handlers. The user could override the default and screw up
> the operation of the globally defined authentication handlers if
> those handlers
> were expecting to find a module by searching that path but by user
> changing
> it, they could no longer be found. But then, in this situation it may
> be better for
> the authentication handler to load the module by an explicit path
> thereby avoiding
> the problem altogether.
>
> Now by using a directive though, it would make it harder for a user
> handler,
> for example one that ran in the fixup phase, to dynamically modify
> the search
> path for modules for all response handlers executed within a certain
> context.
> Specifically, if the search path is set by a PythonOption, then the
> fixup handler
> could actually modify the inherited value of the option by modifying
> the value
> stored in the req.get_options() table object. If the proposed
> PythonAllowOverride
> directive can disallow setting of specific options using
> PythonOption, then a
> good middle ground might be found as one could block the options
> being set
> in a .htaccess file and therefore it would not interfere with a
> global authentication
> handler, but a fixup handler could be allowed to still set it for the
> subsequent
> response handler. But then, I am talking hypotheticals here and maybe
> I am
> just thinking too hard. Thinking up such scenarios though, is what is
> required
> in making good generic reusable modules or frameworks. :-)

Yes, you always put a great deal of thought into the code you write.
That's a good thing. It seems like with both a new directive and a
PythonOption there's advantages and disadvantages, but I'm reticent to
give any advice because I don't understand mod_python the way you do.
Whichever you decide is fine by me.

>
> Is IMPORT_PATH different to or the same as:
>
> r'C:\Docume~1\Dan\MyDocu~1\PYROOT\sidewi
nder\'
>
> Anyway, the other way of doing the above such that a search directory is
> inherited, is to rather than use a module name, translate that to a
> full path and
> import that. The 'path' argument to apache.import_module() can then
> be set
> to be a path embedded as __mp_path__ in the module loaded. Ie. something
> like:
>
> GENERIC_MODULE_PATH = '/some/path/generic'
> CONFIG_MODULE_PATH = '/some/path/config'
>
> GENRIC_MODULE_FILE = os.path.join(GENERIC_MODULE_PATH, "module.py")
>
> module = apache.import_module(GENERIC_MODULE_FILE, path=
> [CONFIG_MODULE_PATH])
>
> The path though is only inherited by the module immediately imported
> though
> and not further.
>

Yes, that's what I'm using, mpath is absolute, but this module is one
of those 'further' ones, and that's why I have to use that workaround.

<snip>
>
> Simpler just to allow a global search path like PythonPath that is
> embedded into
> modules when they are imported. It would be embedded so that its
> value can't
> change over time for a module that has already been imported.
>

Yes, I think that would work best. If you want some help I'm here,
just drop me an email with what you want me to do. Otherwise please
send me an email to let me know when you've got that working.