Re: [development] low hanging fruit for Drupal 6: variable defaults
+1 to centralized defaults. I don't know from localization, so I'll leave it to Gabor to decide what's best for that. The "variable registry" hook seems consistent with what Drupal 6 is doing. We have a menu callback registry (hook_menu), a forms registry (hook_forms), we now have a theme registry (hook_theme), why not a variable registry (hook_variables)? :-) I'm open to either that or mandated variable_set() in .install. I think the registry would be a bit easier for development (easier to add/change quickly), while mandated variable_set() would mean we don't need to add another hook to each module (less code++). I'm not wild about the namespace concept, until I see a code sample of how I'd use it. Would that replace the second parameter? variable_get('system', 'site_name')? Remember that modules call variables across the module-line all the time, so that needs to be kept as simple as possible. As long as we're mucking about in the variable system, the cache system recently introduced a "serialized" flag to specify if a given value needs to be serialized or not. (I believe the default was object/array = serialize else don't bother.) That saves the unserialization time on a string or int. Given that the variable table is loaded completely on a page load, I think the same thing could work here to reduce the amount of deserialization required. I dare say half of the stuff stored there, probably more, is just ints and strings that don't need to be serialized. (And it would make manually setting clean URLs off easier, a common support request. <g>) I'm not sure how that impacts the registry/install question, but consider that a feature request while you're at it. Hm, here's another use case I just thought of... Do we have cases where the default from variable_get() is itself dynamic, such as the return value from another function? If so, is that rare enough that we can say "just use NULL there and throw in your own if statement"? --Larry Garfield On Thu, 3 May 2007 08:15:40 -0700, "John Vandervort" <jvandervort@gmail.com> wrote:
Nice. I hadn't read adrian's thread before. If only I'd searched on 'relm' :)
So if we change the variable table to (system.install):
db_query("CREATE TABLE {variable} ( namespace varchar(128) NOT NULL default '' name varchar(128) NOT NULL default '', value longtext NOT NULL, language varchar(12) NOT NULL default '', PRIMARY KEY (namespace, name, language) ) /*!40100 DEFAULT CHARACTER SET UTF8 */ ");
We'd be halfway there.
a patch to variable_get, set, del, and init... in bootstrap.inc and we'd maintain full backward compatibility with the current system, but allow namespace uniqueness and a helper function for mass loading...
On 5/3/07, Moshe Weitzman <weitzman@tejasa.com> wrote:
- we can save extra typing and value passing on variable_get calls - we have a hook to reuse when we need a list of variables to present for localization
adds pluses over just using constants.
Sure, only using constants for default variable values could be make variable usage better, but I think a hook do more just by being a central place and not a set of unrelated constants.
i'd like to see this centralized in a hook too. we'll be one step closer to the hierarchy that adrian proposed long ago: http://lists.drupal.org/archives/development/2006-06/msg00309.html
-- -> JV
RE: namespaces. variable_get('var_name') variable_get('var_name', 'namespace_name') I think both should be valid. The "" namespace would be for legacy modules or those who don't want namespace. The second would be preferred and allow uniqueness and loading of the entire namespace in a single db call. I just don't think variable name stomping or 'sufficiently large' variable names are a good thing:) On 5/3/07, Larry Garfield <larry@garfieldtech.com> wrote:
+1 to centralized defaults. I don't know from localization, so I'll leave it to Gabor to decide what's best for that.
The "variable registry" hook seems consistent with what Drupal 6 is doing. We have a menu callback registry (hook_menu), a forms registry (hook_forms), we now have a theme registry (hook_theme), why not a variable registry (hook_variables)? :-) I'm open to either that or mandated variable_set() in .install. I think the registry would be a bit easier for development (easier to add/change quickly), while mandated variable_set() would mean we don't need to add another hook to each module (less code++).
I'm not wild about the namespace concept, until I see a code sample of how I'd use it. Would that replace the second parameter? variable_get('system', 'site_name')? Remember that modules call variables across the module-line all the time, so that needs to be kept as simple as possible.
As long as we're mucking about in the variable system, the cache system recently introduced a "serialized" flag to specify if a given value needs to be serialized or not. (I believe the default was object/array = serialize else don't bother.) That saves the unserialization time on a string or int. Given that the variable table is loaded completely on a page load, I think the same thing could work here to reduce the amount of deserialization required. I dare say half of the stuff stored there, probably more, is just ints and strings that don't need to be serialized. (And it would make manually setting clean URLs off easier, a common support request. <g>) I'm not sure how that impacts the registry/install question, but consider that a feature request while you're at it.
Hm, here's another use case I just thought of... Do we have cases where the default from variable_get() is itself dynamic, such as the return value from another function? If so, is that rare enough that we can say "just use NULL there and throw in your own if statement"?
--Larry Garfield
On Thu, 3 May 2007 08:15:40 -0700, "John Vandervort" < jvandervort@gmail.com> wrote:
Nice. I hadn't read adrian's thread before. If only I'd searched on 'relm' :)
So if we change the variable table to (system.install):
db_query("CREATE TABLE {variable} ( namespace varchar(128) NOT NULL default '' name varchar(128) NOT NULL default '', value longtext NOT NULL, language varchar(12) NOT NULL default '', PRIMARY KEY (namespace, name, language) ) /*!40100 DEFAULT CHARACTER SET UTF8 */ ");
We'd be halfway there.
a patch to variable_get, set, del, and init... in bootstrap.inc and we'd maintain full backward compatibility with the current system, but allow namespace uniqueness and a helper function for mass loading...
On 5/3/07, Moshe Weitzman <weitzman@tejasa.com> wrote:
- we can save extra typing and value passing on variable_get calls - we have a hook to reuse when we need a list of variables to present for localization
adds pluses over just using constants.
Sure, only using constants for default variable values could be make variable usage better, but I think a hook do more just by being a central place and not a set of unrelated constants.
i'd like to see this centralized in a hook too. we'll be one step
closer
to the hierarchy that adrian proposed long ago: http://lists.drupal.org/archives/development/2006-06/msg00309.html
-- -> JV
-- -> JV
I'd request/recommend that if all of this goes in drupal 6, we change the function name at the same time. Changing the function this drastically without changing its signature can be a bit dangerous. On May 3, 2007, at 8:56 AM, John Vandervort wrote:
RE: namespaces.
variable_get('var_name') variable_get('var_name', 'namespace_name')
I think both should be valid. The "" namespace would be for legacy modules or those who don't want namespace. The second would be preferred and allow uniqueness and loading of the entire namespace in a single db call.
I just don't think variable name stomping or 'sufficiently large' variable names are a good thing:)
On 5/3/07, Larry Garfield <larry@garfieldtech.com> wrote:
+1 to centralized defaults. I don't know from localization, so I'll leave it to Gabor to decide what's best for that.
The "variable registry" hook seems consistent with what Drupal 6 is doing. We have a menu callback registry (hook_menu), a forms registry (hook_forms), we now have a theme registry (hook_theme), why not a variable registry (hook_variables)? :-) I'm open to either that or mandated variable_set() in .install. I think the registry would be a bit easier for development (easier to add/ change quickly), while mandated variable_set() would mean we don't need to add another hook to each module (less code++).
I'm not wild about the namespace concept, until I see a code sample of how I'd use it. Would that replace the second parameter? variable_get('system', 'site_name')? Remember that modules call variables across the module-line all the time, so that needs to be kept as simple as possible.
As long as we're mucking about in the variable system, the cache system recently introduced a "serialized" flag to specify if a given value needs to be serialized or not. (I believe the default was object/array = serialize else don't bother.) That saves the unserialization time on a string or int. Given that the variable table is loaded completely on a page load, I think the same thing could work here to reduce the amount of deserialization required. I dare say half of the stuff stored there, probably more, is just ints and strings that don't need to be serialized. (And it would make manually setting clean URLs off easier, a common support request. <g>) I'm not sure how that impacts the registry/install question, but consider that a feature request while you're at it.
Hm, here's another use case I just thought of... Do we have cases where the default from variable_get() is itself dynamic, such as the return value from another function? If so, is that rare enough that we can say "just use NULL there and throw in your own if statement"?
--Larry Garfield
On Thu, 3 May 2007 08:15:40 -0700, "John Vandervort" <jvandervort@gmail.com> wrote:
Nice. I hadn't read adrian's thread before. If only I'd searched on 'relm' :)
So if we change the variable table to (system.install):
db_query("CREATE TABLE {variable} ( namespace varchar(128) NOT NULL default '' name varchar(128) NOT NULL default '', value longtext NOT NULL, language varchar(12) NOT NULL default '', PRIMARY KEY (namespace, name, language) ) /*!40100 DEFAULT CHARACTER SET UTF8 */ ");
We'd be halfway there.
a patch to variable_get, set, del, and init... in bootstrap.inc and we'd maintain full backward compatibility with the current system, but allow namespace uniqueness and a helper function for mass loading...
On 5/3/07, Moshe Weitzman <weitzman@tejasa.com > wrote:
- we can save extra typing and value passing on variable_get
calls
- we have a hook to reuse when we need a list of variables to present for localization
adds pluses over just using constants.
Sure, only using constants for default variable values could be make variable usage better, but I think a hook do more just by being a central place and not a set of unrelated constants.
i'd like to see this centralized in a hook too. we'll be one step closer to the hierarchy that adrian proposed long ago: http://lists.drupal.org/archives/development/2006-06/msg00309.html
-- -> JV
-- -> JV
Larry Garfield wrote:
As long as we're mucking about in the variable system, the cache system recently introduced a "serialized" flag to specify if a given value needs to be serialized or not. (I believe the default was object/array = serialize else don't bother.) That saves the unserialization time on a string or int. Given that the variable table is loaded completely on a page load, I think the same thing could work here to reduce the amount of deserialization required. I dare say half of the stuff stored there, probably more, is just ints and strings that don't need to be serialized. (And it would make manually setting clean URLs off easier, a common support request. <g>) I'm not sure how that impacts the registry/install question, but consider that a feature request while you're at it.
Larry, variables are loaded from the cache table as one big blob, and deserialized in bootstrap time. The code is in variable_init() and it does try a cache_get() first. This makes it indepenent of how the variables table is structured. Gabor
But nevertheless it would be a good "administration usability" improvement, and easy to implement. Wim On May 3, 2007, at 18:34 , Gabor Hojtsy wrote:
Larry Garfield wrote:
As long as we're mucking about in the variable system, the cache system recently introduced a "serialized" flag to specify if a given value needs to be serialized or not. (I believe the default was object/array = serialize else don't bother.) That saves the unserialization time on a string or int. Given that the variable table is loaded completely on a page load, I think the same thing could work here to reduce the amount of deserialization required. I dare say half of the stuff stored there, probably more, is just ints and strings that don't need to be serialized. (And it would make manually setting clean URLs off easier, a common support request. <g>) I'm not sure how that impacts the registry/install question, but consider that a feature request while you're at it.
Larry, variables are loaded from the cache table as one big blob, and deserialized in bootstrap time. The code is in variable_init() and it does try a cache_get() first. This makes it indepenent of how the variables table is structured.
Gabor
Indeed, it will still help manual editing of some common variables, so feel free to go for it. Goba Wim Leers wrote:
But nevertheless it would be a good "administration usability" improvement, and easy to implement.
Wim
On May 3, 2007, at 18:34 , Gabor Hojtsy wrote:
Larry Garfield wrote:
As long as we're mucking about in the variable system, the cache system recently introduced a "serialized" flag to specify if a given value needs to be serialized or not. (I believe the default was object/array = serialize else don't bother.) That saves the unserialization time on a string or int. Given that the variable table is loaded completely on a page load, I think the same thing could work here to reduce the amount of deserialization required. I dare say half of the stuff stored there, probably more, is just ints and strings that don't need to be serialized. (And it would make manually setting clean URLs off easier, a common support request. <g>) I'm not sure how that impacts the registry/install question, but consider that a feature request while you're at it.
Larry, variables are loaded from the cache table as one big blob, and deserialized in bootstrap time. The code is in variable_init() and it does try a cache_get() first. This makes it indepenent of how the variables table is structured.
Gabor
participants (5)
-
David Metzler -
Gabor Hojtsy -
John Vandervort -
Larry Garfield -
Wim Leers