Re: mutex dir?
torsdagen den 16 februari 2006 22.48 skrev Jim Gallacher: Oden Eriksson wrote: tisdagen den 14 februari 2006 13.17 skrev Oden Eriksson: Hello. [...] I am no programmer, but can't you just look at how this is handled in the core mod_ssl and ldap code? That would be cheating and no fun at all! ;) Seriously though, we do often look through the source of other modules for ideas and inspiration, but the code bases are different enough that it's not a simple cut and paste. I know it may seem like we've gone off on a tangent, but I think it's better to come up with a general solution rather than creating a bunch of special cases, each of which end up with a slightly different implementation. He he he, I see. Then it wasn't just me who thought this was not a cut and paste thing. I also looked in many other third party modules to see if I could do that :) -- Regards // Oden Eriksson Mandriva: http://www.mandriva.com NUX: http://li.nux.se
Re: mutex dir?
tisdagen den 14 februari 2006 13.17 skrev Oden Eriksson: Hello. [...] I am no programmer, but can't you just look at how this is handled in the core mod_ssl and ldap code? -- Regards // Oden Eriksson Mandriva: http://www.mandriva.com NUX: http://li.nux.se
Re: mutex dir?
Jim Gallacher wrote .. If the settings are going to be a generic key/value like in PythonOption, but only for purposes of the mod_python system itself, maybe it should be called PythonSystemOption. Prefer PythonSystemOption as Module is too confusing to me given you have both Apache modules and Python modules and a module importer. Fair enough. PythonSystemOption is a better. The directive definition is such that it cannot be used in VirtualHost, Directory, Location and so on which should help emphasize that it's use in only for configuring mod_python.so initialization. Also wary of Config because of confusion with req.get_config(). Except that PythonSystemOption just stuffs strings into server-module_config-directives table, which can be accessed via req.server.get_config(). I can't see any reason application code will ever need the settings PythonSystemOption might set, so it's not a big deal if it's not obvious from the name that the settings can be retrieved with get_config. I like the new name as it signals our intention of how PythonSystemOption should be used. This name is also consistent with PythonOption, which is a good idea. I have a better option (pun intended). :-) We do not need a new directive. Instead use existing PythonOption directive. In the handler code for the directive, it can look at the value of the cmd_parms-path and determine if it is being used outside of all VirtualHost, Directory, Location, File directives, ie., at global scope path will be NULL. If it is, then stick it in a table object associated with the server level config for mod_python. This would then be accessible as: req.server.get_options(). Because PythonOption as used now at global scope results in options being seen in req.get_options(), so that still works, we merge the server options into the request one before overlaying it with the request specific options. In other words req.get_options() is just like now, but req.server.get_options() becomes the subset of options which were defined at global server scope. Anything used by mod_python itself would use a namespace prefix of mod_python. as discussed before. Eg. PythonOption mod_python.mutex_directory Thus, existing directive is used and just needs to be stated that option is only used if at global server scope. Graham
Re: mutex dir?
Jim Gallacher wrote ..
I have a better option (pun intended). :-)
We do not need a new directive. Instead use existing PythonOption
directive.
That could work.
In the handler code for the directive, it can look at the
value of the cmd_parms-path and determine if it is being used outside
of all VirtualHost, Directory, Location, File directives, ie., at global
scope path will be NULL.
The disadvantage is that every request which triggers the
directive_PythonOption call would require a number of string
comparisons. Granted this is done in C, but there is a penalty to be paid.
No. No string comparisons at all. Check that cmd-path == NULL only.
If it is, then stick it in a table object associated
with the server level config for mod_python. This would then be accessible
as: req.server.get_options().
Do you see us adding a new apr_table_t field to the py_config structure
(define in mod_python.h)? Or would these server options go in either the
directives or options table?
Hmmm, this is interesting. It seems that the code may have intended to
implement the exact feature I was talking about but it wasn't completed.
The code has:
static const char *directive_PythonOption(cmd_parms *cmd, void * mconfig,
const char *key, const char *val)
{
py_config *conf;
conf = (py_config *) mconfig;
if(val!=NULL) {
apr_table_set(conf-options, key, val);
conf = ap_get_module_config(cmd-server-module_config,
python_module);
apr_table_set(conf-options, key, val);
}
else {
/** We don't remove the value, but set it
to an empty string. There is no possibility
of colliding with an actual value, since
an entry string precisely means 'remove the value' */
apr_table_set(conf-options, key, );
conf = ap_get_module_config(cmd-server-module_config,
python_module);
apr_table_set(conf-options, key, );
}
return NULL;
}
It was already setting the options table object in the server config
object, but was doing it no matter where in config. Thus no different
to request object. If instead it is written as:
static const char *directive_PythonOption(cmd_parms *cmd, void * mconfig,
const char *key, const char *val)
{
py_config *conf;
conf = (py_config *) mconfig;
if(val!=NULL) {
apr_table_set(conf-options, key, val);
if (cmd-path == NULL) {
conf = ap_get_module_config(cmd-server-module_config,
python_module);
apr_table_set(conf-options, key, val);
}
}
else {
/** We don't remove the value, but set it
to an empty string. There is no possibility
of colliding with an actual value, since
an entry string precisely means 'remove the value' */
apr_table_set(conf-options, key, );
if (cmd-path == NULL) {
conf = ap_get_module_config(cmd-server-module_config,
python_module);
apr_table_set(conf-options, key, );
}
}
return NULL;
}
Then it then correctly separates the global from the non global.
The merging code already merges the two back together to
construct the actual request object.
Code will actually run quicker than before, as have avoided inserting
values into two tables when not global config.
The only thing then missing is in serverobject.c, which needs:
/**
** server.get_options(server self)
**
* Returns the options set through PythonOption directives.
* unlike req.get_options, this one returns the per-server config
*/
static PyObject * server_get_options(serverobject *self)
{
py_config *conf =
(py_config *) ap_get_module_config(self-server-module_config,
python_module);
return MpTable_FromTable(conf-options);
}
static PyMethodDef server_methods[] = {
{get_config, (PyCFunction) server_get_config,
METH_NOARGS},
{get_options, (PyCFunction) server_get_options,
METH_NOARGS},
{register_cleanup, (PyCFunction) server_register_cleanup,
METH_VARARGS},
{ NULL, NULL } /* sentinel */
};
If I then have at global config scope:
PythonOption global 1
PythonOption override 1
and in a .htaccess file:
PythonOption local 1
PythonOption override 2
The end result is:
req.get_options()={'local': '1', 'override': '2', 'global': '1'}
req.server.get_options()={'override': '1', 'global': '1'}
How does req.server.get_options() differ from req.server.get_config(),
which already exists?
I still see what is in get_config() as special, ie., the values for
actual directives. Just don't think it is good to mix them.
And lest we forget the original intent of this
discussion,
Re: mutex dir?
Graham Dumpleton wrote ..
Graham Dumpleton wrote ..
How does req.server.get_options() differ from req.server.get_config(),
which already exists?
I still see what is in get_config() as special, ie., the values for
actual directives. Just don't think it is good to mix them.
Looking at this further, the distinction between req.get_config() and
req.server.get_config() seems to be all broken. The code for PythonDebug
is:
/**
** directive_PythonDebug
**
* This function called whenever PythonDebug directive
* is encountered.
*/
static const char *directive_PythonDebug(cmd_parms *cmd, void *mconfig,
int val) {
const char *rc = python_directive_flag(mconfig, PythonDebug, val,
0);
if (!rc) {
py_config *conf = ap_get_module_config(cmd-server-module_config,
python_module);
return python_directive_flag(conf, PythonDebug, val, 0);
}
return rc;
}
The python_directive_flag() function always returns NULL and so it adds
the config value to both table objects. This means that local values for
the directives in a directory/location/files/host context are going to
overwrite the global ones in req.server.
This code should perhaps similarly be looking to see if cmd-path is
NULL.
Thus, FWIW, I get what I would expect when I change the code to:
/**
** directive_PythonDebug
**
* This function called whenever PythonDebug directive
* is encountered.
*/
static const char *directive_PythonDebug(cmd_parms *cmd, void *mconfig,
int val) {
const char *rc = python_directive_flag(mconfig, PythonDebug, val, 0);
if (!cmd-path) {
py_config *conf = ap_get_module_config(cmd-server-module_config,
python_module);
return python_directive_flag(conf, PythonDebug, val, 0);
}
return rc;
}
It isn't just this directive processor though, all of them should have:
if (!rc) {
changed to:
if (!cmd-path) {
The actual return values from the function are really meaningless as
they are always NULL.
Graham
mutex dir?
Hello. In our package in Mandriva I patch mod_python.c so that the mutex stuff is put in /var/cache/httpd/mod_python/. But now with mod_python-3.2.7 plus fixes from the trunk and running the test suite it complains it cannot access /var/cache/httpd/mod_python/ (of course). So my question/request is, could you please make this directory set in the config? -- Regards // Oden Eriksson Mandriva: http://www.mandriva.com NUX: http://li.nux.se
Re: mutex dir?
On Tue, 14 Feb 2006, Jim Gallacher wrote: I wonder if we should generalize this, so rather than PythonMutexDir, we have PythonModuleConfig. Usage might look like: PythonModuleConfig mutex_dir /path/to/mutexs PythonModuleConfig max_mutex_locks 8 I may be wrong, but I think the reason this was never configurable was because the mutex is created before the apache config is read. Grisha
Re: mutex dir?
Oden Eriksson wrote: Hello. In our package in Mandriva I patch mod_python.c so that the mutex stuff is put in /var/cache/httpd/mod_python/. But now with mod_python-3.2.7 plus fixes from the trunk and running the test suite it complains it cannot access /var/cache/httpd/mod_python/ (of course). So my question/request is, could you please make this directory set in the config? I assume you mean as an option to configure, rather than as an Apache configuration directive? Jim
Re: mutex dir?
tisdagen den 14 februari 2006 14.19 skrev Jim Gallacher: Oden Eriksson wrote: Hello. In our package in Mandriva I patch mod_python.c so that the mutex stuff is put in /var/cache/httpd/mod_python/. But now with mod_python-3.2.7 plus fixes from the trunk and running the test suite it complains it cannot access /var/cache/httpd/mod_python/ (of course). So my question/request is, could you please make this directory set in the config? I assume you mean as an option to configure, rather than as an Apache configuration directive? I meant as an apache configuration directive. -- Regards // Oden Eriksson Mandriva: http://www.mandriva.com NUX: http://li.nux.se
Re: mutex dir?
Oden Eriksson wrote: tisdagen den 14 februari 2006 14.19 skrev Jim Gallacher: Oden Eriksson wrote: Hello. In our package in Mandriva I patch mod_python.c so that the mutex stuff is put in /var/cache/httpd/mod_python/. But now with mod_python-3.2.7 plus fixes from the trunk and running the test suite it complains it cannot access /var/cache/httpd/mod_python/ (of course). So my question/request is, could you please make this directory set in the config? I assume you mean as an option to configure, rather than as an Apache configuration directive? I meant as an apache configuration directive. After giving this some thought I think it should be both, so the options become: 1. Default /tmp 2. --configure --with-mutex-dir=/some/directory Allows distributions to package mod_python according to their platform specification, without the need for applying any patches. 3. PythonMutexDir /some/place This would mainly be used in the unit tests to override the setting applied by option #2. This avoids Oden's unit test problem which is similar to the problem described in MODPYTHON-119, where the unit test defaults may conflict with a mod_python instance running on the server. I wonder if we should generalize this, so rather than PythonMutexDir, we have PythonModuleConfig. Usage might look like: PythonModuleConfig mutex_dir /path/to/mutexs PythonModuleConfig max_mutex_locks 8 Currently the number of mutex locks is set with ./configure --with-max-locks By the way Oden, are you the offical mod_python maintainer on Mandriva? Jim
