I find that on a few occasions, I've needed to generate extra SQL for both creation and deletion tasks when I'm executing "manage.py syncdb" (or sql, or reset, sqlclear, etc., pretty much all of the SQL generation commands). Currently, if you use only use syncdb for everything, you can do this in a very limited way. I have a ticket and patch already in Trac for adding a signal to reset that would work the same way as the syncdb signal.
But, these cases still wouldn't allow you to just print this additionally generated SQL, or in any way retrieve and use it from any of the other commands (sql, sqlclear, etc.). This is sometimes frustrating, as these additional SQL statements being generated are dependent on the current state of the model.
I know that "sqlcustom" is available for adding SQL to things, but that only reads in static files, which is no good if the custom SQL you wish to generate is specific to the app's models as they stand at the time they're run.
So, my proposal is this: generate hooks for users. For each of the get_custom_sql/get_create_sql/etc., add a small portion that checks the installed apps for their own management.py file and an appropriate method. For instance, "sqlcustom"'s method could be "get_custom_sql_for_model_all", denoting that it's run on every model in every app that is having the current manage.py operation applied to it. These functions would be expected to return an array of SQL statements, which could then be fit in with the other generated SQL from each of the current built in methods.
For clarity, here's an example (with existing statements left in the method to show where it might be dropped):
(from django/core/management.py) def get_custom_sql_for_model(model): ... # Some backends can't execute more than one SQL statement at a time, # so split into separate statements. statements = re.compile(r";[ \t]*$", re.M)
# Generate custom SQL from each app's management.py, if it is designed to # process all models. for app_name in settings.INSTALLED_APPS: try: app_management = __import__(app_name + '.management', {}, {}, ['']) if 'get_custom_sql_for_model_all' in dir(app_management): output += app_management.get_custom_sql_for_model_all(model) except ImportError: pass
# Find custom SQL, if it's available. ...
(an example of <some app>/management.py) def get_custom_sql_for_model_all(model): statements = ['SELECT 1', 'SELECT 2'] return statements
This additional block works much in the way that a registered signal works from syncdb. For every model, all the apps are checked for a management.py that contains a "get_custom_sql_for_model_all" method. If the method is found, it's run for the current model. (This also allows you to have one app that can contain SQL modifiers for all the installed apps in the system, a HUGE boon to adding functionality project-wide). Also note that this breaks no existing functionality and creates no extra dependencies, since anything that is missing is ignored.
How do people feel about this? I will happily generate a patch that includes all the appropriate methods, not just get_cutom_sql(), but I want to make sure that this is something that would likely be accepted into the codebase before doing so.
On 8/9/07, George Vilches <g...@thataddress.com> wrote:
> But, these cases still wouldn't allow you to just print this > additionally generated SQL, or in any way retrieve and use it from any > of the other commands (sql, sqlclear, etc.). This is sometimes > frustrating, as these additional SQL statements being generated are > dependent on the current state of the model.
+1. I love this idea! As a side note, it actually fits in with a whole lot of other ideas that have been circulating recently.
- Speeding up the test system with a database mock will require the ability to record and replay the commands going to the database.
- The schema-evolution suggestions that are under development may require the ability to convert a sequence of python-based ORM commands into their equivalent SQL for storage, rather than execution directly on the backend.
> So, my proposal is this: generate hooks for users. For each of the > get_custom_sql/get_create_sql/etc., add a small portion that checks the > installed apps for their own management.py file and an appropriate > method. For instance, "sqlcustom"'s method could be > "get_custom_sql_for_model_all", denoting that it's run on every model in > every app that is having the current manage.py operation applied to it. > These functions would be expected to return an array of SQL > statements, which could then be fit in with the other generated SQL from > each of the current built in methods.
This is actually how the management commands started out - once upon a time, you ran ./manage.py install myapp, which was a wrapper around calling ./manage.py sqlall myapp and piping the output to the database.
The problem is that this approach isn't very flexible. Some of what syncdb does isn't handled at a raw SQL level - we use the ORM to generate the commands/queries etc. post_sync handlers, for example, would be almost impossible to recode in the way you describe, and any user-based post_sync handlers would need to support some sort of 'retrieve sql' API.
I would suggest approaching this problem at lower than that - installing a filter at the level of the database cursor that diverts queries away from the actual database, and into a store. That way, if you run the code 'Author.objects.all()' , the backend will get the request to invoke 'SELECT * FROM Author', but this will get recorded rather than sent to the database.
You then add a --sql flag to ./manage.py that sets up the recording mode on the database backend, and outputs the command buffer at the end of execution. If you make this interface generic, anyone could invoke SQL recording whenever they want.
Part of this infrastructure is already in place for logging purposes. Improvements to the logging capability that allow for recording and playback would be most welcome.
Russell Keith-Magee wrote: > I would suggest approaching this problem at lower than that - > installing a filter at the level of the database cursor that diverts > queries away from the actual database, and into a store. That way, if > you run the code 'Author.objects.all()' , the backend will get the > request to invoke 'SELECT * FROM Author', but this will get recorded > rather than sent to the database.
> You then add a --sql flag to ./manage.py that sets up the recording > mode on the database backend, and outputs the command buffer at the > end of execution. If you make this interface generic, anyone could > invoke SQL recording whenever they want.
> Part of this infrastructure is already in place for logging purposes. > Improvements to the logging capability that allow for recording and > playback would be most welcome.
How about the patch below? When you create the cursor, if you want access to "don't run this SQL, just have playback available", just use connection.cursor(playback_only=True), and if you want to roll the playback, you can either use self.db.queries directly, or use the util.CursorDebugWrapper.playback method. Totally backwards-compatible and shouldn't step on any toes (uses existing logging, like you mentioned above). You can easily add this into django/core/management.py. If this is okay, I'll do a patch for all the existing backends, it's a simple change now that I've looked through everything, assuming there's no changes related to the next paragraph.
Something that I found a little irksome while working on this is that the DatabaseWrapper class for each backend doesn't inherit from some logical parent. I know that all the db-specific functionality is wrapped by these classes, but there are things that are in each class that are most definitely shared functionality, like the mechanism by which util.CursorDebugWrapper is instantiated. We could move that off to a method in a base class very nicely, and then the playback_only addition wouldn't have to be added to every backend, or the change would be more minimal. Also, I think it might be appropriate to have "playback()" be a method of the DatabaseWrapper (connection) and not the CursorDebugWrapper, since if there's any DB-specific separators we need to use, there's no way to deal with that from the cursor. But, if we don't make a base class, then we're going to have to duplicate playback's code in all the backend/(db)/base.py files. Not very DRY.
Discussion subject changed to "Changing the options in manage.py, adding more signals (was Re: Adding hooks to methods that generate SQL in django/core/management.py)" by George Vilches
I'm splitting this conversation off from the other conversation on SQL logging, because I think that topic has merits all its own, and this one has more to discuss and slightly more possibility for backwards-incompatibility. I don't want to pork barrel this issue in with a perfectly legitimate one. :)
Russell Keith-Magee wrote: > On 8/9/07, George Vilches <g...@thataddress.com> wrote: >> So, my proposal is this: generate hooks for users. For each of the >> get_custom_sql/get_create_sql/etc., add a small portion that checks the >> installed apps for their own management.py file and an appropriate >> method. For instance, "sqlcustom"'s method could be >> "get_custom_sql_for_model_all", denoting that it's run on every model in >> every app that is having the current manage.py operation applied to it. >> These functions would be expected to return an array of SQL >> statements, which could then be fit in with the other generated SQL from >> each of the current built in methods.
> This is actually how the management commands started out - once upon a > time, you ran ./manage.py install myapp, which was a wrapper around > calling ./manage.py sqlall myapp and piping the output to the > database.
> The problem is that this approach isn't very flexible. Some of what > syncdb does isn't handled at a raw SQL level - we use the ORM to > generate the commands/queries etc. post_sync handlers, for example, > would be almost impossible to recode in the way you describe, and any > user-based post_sync handlers would need to support some sort of > 'retrieve sql' API.
What I was proposing didn't involve taking the SQL code that Django generates and modifying it. Rather, this would allow the user to add more SQL that they generate entirely independently from what Django's internals are generating, but based on contents in the app (as opposed to static SQL files that can be loaded in).
Example: Say I'm building a dynamic model. Say that, for whatever reason, I want this dynamic model to have a DB backing, just like any Django model. (I know there's already comments on the wiki as to the gotchas of this and why this may not be a great example, but it's the easiest one for me to explain). Well, syncdb works fine, if I have a signal to dispatch. But say I want to use something that doesn't have a signal, like "sql" or "sqldelete". I have no way to get my app's management method executed.
So possibly, we get rid of "sql", "sqldelete" and the like, and only have "create", "delete", etc., which actually do the task, and like you said, have a --sql flag that does output only. If we do this, we still need to address how to call the user-level management.py code to make sure that all the extra user SQL is *always* included in every possible way the manage.py can be run.
Here's the two options I see:
1) Add a signal to every option? Right now, only syncdb has a signal, although I have a ticket and patch for adding a signal to reset, see http://code.djangoproject.com/ticket/5065 . Doing this would be pretty straightforward, shouldn't break anyone's existing code (since we wouldn't be removing the syncdb signal, we'd just be making it more granular). Although, if we have a create signal and a delete signal, we'd need to make sure that when running a syncdb we don't also fire those signals. Easy enough, just something to be careful for.
2) Add a callback to every option? This is similar to the example I wrote in the first message. Less married to this with the idea of SQL logging and playback.
My vote is for 1), I think with the playback feature, it would be pretty sexy, give us more functionality than is currently in manage.py, and fewer keywords to do so. I'll happily write the patch for it if there's no objections to the idea. The only thing that the idea currently breaks would be some of the old "manage.py" keywords, but the whole point of adding the logger and a --sql flag was to reduce the number of useless keywords we have in manage.py, which I'm fully +1 for. Too many redundant/easily merge-able options in there. :)
On Sun, Aug 12, 2007 at 09:20:06AM -0400, George Vilches wrote: > Something that I found a little irksome while working on this is that > the DatabaseWrapper class for each backend doesn't inherit from some > logical parent. I know that all the db-specific functionality is > wrapped by these classes, but there are things that are in each class > that are most definitely shared functionality, like the mechanism by > which util.CursorDebugWrapper is instantiated.
Might want to take a look at ticket 5106; was working on connection pooling earlier this week, but ran into the same "wow, backend code needs some serious refactoring"- will push the patch later today, but basically have shifted everything so that construction, inspection, and client bits all are bound to the actual database wrapper instance itself- via that, can use a generic connection pooler that just wraps the real db wrapper. You would be able to do the same thing I suspect- just pick out the correct method, and insert the logging there.
On 8/12/07, George Vilches <g...@thataddress.com> wrote:
> How about the patch below? When you create the cursor, if you want > access to "don't run this SQL, just have playback available", just use > connection.cursor(playback_only=True), and if you want to roll the
This is a simple approach that would probably suffice for management.py based problems, but it isn't really practical on a large scale. For example, QuerySets open their own connections, so it won't be easy to pass an argument to an ORM call to enable capture of playback data.
The configuration option will need to be a little more generic - i.e., putting the entire backend into a record mode - not just a single cursor call.
> Something that I found a little irksome while working on this is that > the DatabaseWrapper class for each backend doesn't inherit from some > logical parent. I know that all the db-specific functionality is
Brian Ferring seems to be on top of this one. There's a separate thread now talking about this refactoring.
Russell Keith-Magee wrote: > On 8/12/07, George Vilches <g...@thataddress.com> wrote: >> How about the patch below? When you create the cursor, if you want >> access to "don't run this SQL, just have playback available", just use >> connection.cursor(playback_only=True), and if you want to roll the
> This is a simple approach that would probably suffice for > management.py based problems, but it isn't really practical on a large > scale. For example, QuerySets open their own connections, so it won't > be easy to pass an argument to an ORM call to enable capture of > playback data.
> The configuration option will need to be a little more generic - i.e., > putting the entire backend into a record mode - not just a single > cursor call.
Alright, that makes sense. So, my thought is, if we can't change the call to connection.cursor() on each instantiation, we're going to need some sort of static variable store that can get updated at runtime (or some global). I know that settings is right out for anything that changes at runtime, is there a Django-friendly way of storing a variable of this nature? If we can store this variable in some more non-code changing way, the rest of the patch should be fine, right? Here's my two thoughts for hopefully Django-friendly approaches:
First, Brian Ferring's base class could have a static or class variable easily enough, but I'd like this landing not to depend on that refactor, since that could be a while coming.
Second, we could add a class level variable to each DatabaseWrapper, since the handle to those seem to be instantiated only once at runtime (at least, my short testing with just the Django built-in webserver seemed to do so, I only assume that for Apache it's once per thread). That would be an easy enough variable to update from pretty much anywhere in the app:
connection.playback_only = True
*should* be all we would need, anywhere in the app. If need be, we can add a helper to disguise this call, but every connection.cursor() creation at that would just need to check for self.playback_only to get the same effect as in the previously sent patch.
On 8/13/07, George Vilches <g...@thataddress.com> wrote:
> Russell Keith-Magee wrote: > > The configuration option will need to be a little more generic - i.e., > > putting the entire backend into a record mode - not just a single > > cursor call.
> Second, we could add a class level variable to each DatabaseWrapper, > since the handle to those seem to be instantiated only once at runtime > (at least, my short testing with just the Django built-in webserver > seemed to do so, I only assume that for Apache it's once per thread). > That would be an easy enough variable to update from pretty much > anywhere in the app:
> connection.playback_only = True
There's another possibility you haven't considered - dynamically replacing/wrapping the connection object. The test system already does this for the email framework - when the test framework is set up, the email framework is dynamically replaced with a mock; when the test framework is torn down, the mock is uninstalled. A similar approach could be used to 'start/end SQL recording'.
Discussion subject changed to "Changing the options in manage.py, adding more signals (was Re: Adding hooks to methods that generate SQL in django/core/management.py)" by Russell Keith-Magee
On 8/12/07, George Vilches <g...@thataddress.com> wrote:
> 1) Add a signal to every option?
If we were going to go down this path, this would be the preferred option. However, I'm not sure I'm convinced of the need. Which commands exactly do you think require signals?
Russell Keith-Magee wrote: > On 8/12/07, George Vilches <g...@thataddress.com> wrote: >> 1) Add a signal to every option?
> If we were going to go down this path, this would be the preferred > option. However, I'm not sure I'm convinced of the need. Which > commands exactly do you think require signals?
Let's make the assumption that we can land the changes to manage.py that would allow us to merge the SQL logger and the actual execution commands into a single keyword, so that things like "sqlall" or "sqlreset" would not be necessary, just their equivalent executions. (We can easily map this list backwards if we have to). Then, the commands that would need signals are (with a few footnotes):
(merge sqlflush) flush [--verbosity] [--noinput]
loaddata [--verbosity] fixture, fixture, ...
(merge sqlreset) reset [--noinput][appname ...]
(make this executable?) sql [appname ...]
syncdb [--verbosity] [--noinput] Create the database tables for all apps in INSTALLED_APPS whose tables haven't already been created.
(we would probably want to keep the following, but have a version that's executable as well as printable) sqlall [appname ...]
sqlclear [appname ...]
sqlcustom [appname ...]
sqlindexes [appname ...]
As far as the need goes, here's a generic example. Say I need to create a few extra tables for clever caching (something more dense than just good indexing) as an extension to my Django app, and those tables are very specific to the structure of the model (and as the model changes, so will the contents of those tables). Therefore, I need something that would generate proper CREATE TABLE and DROP TABLE commands and indexes and such, and it would be highly preferable that when I reset my app, I can also reset my additional work that's directly correlated with my app.
I know there is a custom SQL option, but there's no good way to have the DROP and the CREATE (or the create indexes, etc.) isolated from one another, since the custom SQL option doesn't support reading in one of a set of files. And anyway, custom SQL static files wouldn't be very good for this, because we want the contents of the table to be accurate to the Model at this moment in time, and having to generate that statically when we have an awesome Django framework to tie into to do this with doesn't seem very DRY. :)
George Vilches wrote: > Russell Keith-Magee wrote: >> On 8/12/07, George Vilches <g...@thataddress.com> wrote: >>> 1) Add a signal to every option? >> If we were going to go down this path, this would be the preferred >> option. However, I'm not sure I'm convinced of the need. Which >> commands exactly do you think require signals?
Since the first example I gave may not be particularly compelling, since some craftiness with static Django models could be used to solve the problem, let me give one that I don't believe could be solved that way.
Assume I'm building a row-based audit system. I also want this audit system to have one audit table/model per legitimate Django model. So, say I have an app.model called "wiki.article". This would create a "wiki_article" table. I also want to have a "wiki_article_audit" table keeping a full history of changes. Now, since Django models don't support inheritance yet, and I don't want to have to re-create every model that I want to perform an audit on, I can instead create a dynamic model from the original model with a small helper. Unfortunately, syncdb and the like don't have a way of detecting this dynamic model and creating tables and such for it. However, I've already got a mechanism in syncdb (via signals) which uses the existing management functions to write that new dynamic model to the database, and then in runtime everything works perfectly happy.
Unfortunately, since there's only a syncdb signal, I can't even do things like a reset on it, and there's definitely no way currently to get the SQL generated from my syncdb signal. Being able to get the CREATE and DROP statements in text as well as each individually would be a huge boon to this type of use (and any dynamic model use in general).
This sounds like a far more complicated example than I had considered when I was doing my work with dynamic models[1], but I did have success getting syncdb to install dynamic models, provided a few things are in order. I probably didn't document them well enough on the wiki, but I could do so if this is a real need you have.
I also can't speak for how well your audit example would work on the whole using that method, but if it's a real task for somebody, I'd love to help work it out. In theory though, given my past experience, it would be possible to do in such a way that a single line in each audit-enabled model would trigger all the hard work, enabling syncdb and even admin integration.
Keep in mind that I have no opinion on the real meat of this thread, I'm just chiming in to help clarify what is and isn't possible with dynamic models.
> George Vilches wrote: > > Russell Keith-Magee wrote: > >> On 8/12/07, George Vilches <g...@thataddress.com> wrote: > >>> 1) Add a signal to every option? > >> If we were going to go down this path, this would be the preferred > >> option. However, I'm not sure I'm convinced of the need. Which > >> commands exactly do you think require signals?
> Since the first example I gave may not be particularly compelling, since > some craftiness with static Django models could be used to solve the > problem, let me give one that I don't believe could be solved that way.
> Assume I'm building a row-based audit system. I also want this audit > system to have one audit table/model per legitimate Django model. So, > say I have an app.model called "wiki.article". This would create a > "wiki_article" table. I also want to have a "wiki_article_audit" table > keeping a full history of changes. Now, since Django models don't > support inheritance yet, and I don't want to have to re-create every > model that I want to perform an audit on, I can instead create a dynamic > model from the original model with a small helper. Unfortunately, > syncdb and the like don't have a way of detecting this dynamic model and > creating tables and such for it. However, I've already got a mechanism > in syncdb (via signals) which uses the existing management functions to > write that new dynamic model to the database, and then in runtime > everything works perfectly happy.
> Unfortunately, since there's only a syncdb signal, I can't even do > things like a reset on it, and there's definitely no way currently to > get the SQL generated from my syncdb signal. Being able to get the > CREATE and DROP statements in text as well as each individually would be > a huge boon to this type of use (and any dynamic model use in general).
Marty Alchin wrote: > This sounds like a far more complicated example than I had considered > when I was doing my work with dynamic models[1], but I did have > success getting syncdb to install dynamic models, provided a few > things are in order. I probably didn't document them well enough on > the wiki, but I could do so if this is a real need you have.
> I also can't speak for how well your audit example would work on the > whole using that method, but if it's a real task for somebody, I'd > love to help work it out. In theory though, given my past experience, > it would be possible to do in such a way that a single line in each > audit-enabled model would trigger all the hard work, enabling syncdb > and even admin integration.
> Keep in mind that I have no opinion on the real meat of this thread, > I'm just chiming in to help clarify what is and isn't possible with > dynamic models.
That page was a great start, that's where I started to figure most of it out when I started down this path a while back. And what I described is a fully working app, but I've not exposed it because I'm not so sure that it really fits into the spirit of Django, even on the contrib side. :) That having been said, it's something we very much needed for our current app, and the current branch in the Django trunk for doing history just doesn't have the performance for large DBs, especially with large change counts (one table in columnar key/value fashion just won't cut it, especially for reporting purposes). We had to have row-based to do reporting on millions of historical entries at any speed. (And it's also much faster for re-constructing history at the DB level, we can recreate any table at any point in time with a roll-forward type approach from the audit tables, columnar requires a lot more processing to do the same).
As far as how well it works, it's great across the board. :) We have the syncdb signal stuff working fine (all the missing tables are created happily, and I don't have to write any custom SQL, I just piggyback on the things in django/core/management which do a fine job, since a Model is a Model, dynamic or not), and it's actually really solid under load, and easy to add into an existing model to "turn on" auditing. Here's an example of how you use it:
class SomeModel(models.Model): c1 = models.CharField(maxlength=10) c2 = models.ForeignKey(SomeOtherModel)
class Audit: pass
That's it. I think it's nifty. :) Calling save() or delete() on the model automagically writes the audit entries and all the related tasks, all you have to do is add the Audit app to the INSTALLED_APPS, and all the rest is handled, and it doesn't require any hackery to the Django codebase, which makes it near perfect for my uses. :)
So, the problem itself is actually solved for the runtime portion of the app. The *only* thing that I found myself missing is the ability to generate the correct DROP TABLE/CREATE TABLE/CREATE INDEX type SQL in a printable manner that the Django manage.py commands could hook into and either display or run. When I reset an app, I want to reset all my audit tables in that app as well, and there's just no signals in place (and no way to inject SQL code for display even if the signals were there, like in syncdb). Same for "manage.py sql", "manage.py sqlclear", and etc. So, that's at least part of where this all came from, and all the more reason that I like the --sql flag and reducing the manage.py set of SQL-related options to something more straightforward.
On 8/14/07, George Vilches <g...@thataddress.com> wrote:
> Thoughts?
Absolutely! Though I think this thread would get too far off-topic if I rattle them all off right now. I'll do a little work on the subject and see what I can come up with. Your approach looks very close to what I was thinking, but I hadn't planned on relying on the syncdb at all. I have a different approach in mind that should (in theory, at least) solve the problems you're facing. More on that to come.
Okay, I'll post one last time on this subject, then leave you guys to do what you're supposed to be doing. I'm only posting here in case who heard me ranting find it interesting. I did manage to come up with a solution like I mentioned, and I'll be posting it soon. It'd be up already, but it seems djangosnippets.org is having PostgreSQL problems at the moment. I'll be putting it there, writing about it on my blog, and linking to it from the DynamicModels article.
So, for the record, it can be done, retaining syncdb/sql/sqlall/reset functionality for all the dynamic models with very little hackery (the whole of it is barely over 100 lines). Details to follow for those who are interested.
Marty Alchin wrote: > Okay, I'll post one last time on this subject, then leave you guys to > do what you're supposed to be doing. I'm only posting here in case who > heard me ranting find it interesting. I did manage to come up with a > solution like I mentioned, and I'll be posting it soon. It'd be up > already, but it seems djangosnippets.org is having PostgreSQL problems > at the moment. I'll be putting it there, writing about it on my blog, > and linking to it from the DynamicModels article.
> So, for the record, it can be done, retaining syncdb/sql/sqlall/reset > functionality for all the dynamic models with very little hackery (the > whole of it is barely over 100 lines). Details to follow for those who > are interested.
> -Gul
Very much looking forward to it. I think there's still some legitimacy to updating the options in manage.py and adding a --sql flag that supports direct logging (a subtopic in the other thread), but if this functionality can be garnered in the meantime without that patch and without modifying Django internals, all the better!
Discussion subject changed to "Changing the options in manage.py, adding more signals (was Re: Adding hooks to methods that generate SQL in django/core/management.py)" by Russell Keith-Magee
On 8/14/07, George Vilches <g...@thataddress.com> wrote:
> George Vilches wrote: > Unfortunately, since there's only a syncdb signal, I can't even do > things like a reset on it, and there's definitely no way currently to > get the SQL generated from my syncdb signal. Being able to get the > CREATE and DROP statements in text as well as each individually would be > a huge boon to this type of use (and any dynamic model use in general).
> Is that a more reasonable example?
That maybe explains the need for a signal on reset - except that I'd be an advocate for removing the reset command altogether. It's pretty badly broken as it currently stands - any tables with m2m relations will have some major difficulties using it (if you're using a database with row referential integrity, anyway).
Getting access to the CREATE and DROP statements is a separate issue - and was the reason for my suggesting the SQL logger. The idea would be that you could call _any_ manage.py command with the --sql flag, and you would get the SQL output to the screen, rather than the database.
Russell Keith-Magee wrote: > On 8/13/07, George Vilches <g...@thataddress.com> wrote: >> Russell Keith-Magee wrote: >>> The configuration option will need to be a little more generic - i.e., >>> putting the entire backend into a record mode - not just a single >>> cursor call. >> Second, we could add a class level variable to each DatabaseWrapper, >> since the handle to those seem to be instantiated only once at runtime >> (at least, my short testing with just the Django built-in webserver >> seemed to do so, I only assume that for Apache it's once per thread). >> That would be an easy enough variable to update from pretty much >> anywhere in the app:
>> connection.playback_only = True
> There's another possibility you haven't considered - dynamically > replacing/wrapping the connection object. The test system already does > this for the email framework - when the test framework is set up, the > email framework is dynamically replaced with a mock; when the test > framework is torn down, the mock is uninstalled. A similar approach > could be used to 'start/end SQL recording'.
Since Adrian has done great work tonight landing Brian Harring's database refactoring, it seemed like a good time to revisit this, because dropping the addition of "non-executing" or "playback only" SQL (what do we want to call this?) will be in a single place and very painless.
As far as dynamically wrapping it, that's a neat idea, and I think it would have been very appropriate before the refactor. Now, we've already got a debug cursor, and we're already tracking queries, so the change seems very natural.
The two needed file patches are at the bottom of the e-mail. This change is small and totally non-destructive to existing apps. If you want to turn your DB to playback_only, you just have to call:
from django.db import connection connection.playback_only = True
And since the connection variable are only be initialized once per runtime, setting it once anywhere means that the app is in playback only mode for the rest of the run. This could easily be controlled from a middleware, so all the items below this middleware could be playback only, but outer middlewares can still write to the database if necessary.
The only way this could be simpler or easier for the user to control is if there was an entry in settings.py that the user could also use. settings.DATABASE_PLAYBACK_ONLY, maybe? I'm only +0 on this. It would have to be optional, because I can definitely see a reason for wanting to turn this ability on and off during runtime without changing the code at all. Specifically, I could imagine several setup programs in the style of phpBB (I know, it's a sin to mention PHP apps on here, but everything I can think of right this second is in PHP) that might want to turn it off based on whether the user asked to display the queries or actually execute them on install on the previous web page.
|
