Django

Code

Ticket #5106 (closed: fixed)

Opened 1 year ago

Last modified 1 year ago

django.db.backend.*.base refactoring

Reported by: Brian Harring <ferringb@gmail.com> Assigned to: mtredinnick
Milestone: Component: Database layer (models, ORM)
Version: SVN Keywords:
Cc: Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

Splitting connection pooling up into two patches; this particular ticket holds refactoring of django.db.backend.*.base.DatabaseWrapper? classes to remove redundancy, and give an easier spot to control connection pooling.

Very least, the misc backend implementation are a bit redundant- deriving from a common base class is a bit saner in this case for debugging and extension reasons.

Attachments

backend-refactoring.patch (15.5 kB) - added by Brian Harring <ferringb@gmail.com> on 08/06/07 18:11:18.
backend-refactoring-v2.patch (70.3 kB) - added by Brian Harring <ferringb@gmail.com> on 08/13/07 04:26:11.
version/stage 2 of the refactoring; api breaking, but molds existing old style layout into new automatically, and allows for actual OOP trickery.
backend-refactoring-v2-correct-bitrot.patch (72.2 kB) - added by Brian Harring <ferringb@gmail.com> on 08/17/07 18:24:32.
re-cut v2, since adrians core/management horked v2 from being applied.

Change History

08/06/07 18:11:18 changed by Brian Harring <ferringb@gmail.com>

  • attachment backend-refactoring.patch added.

08/07/07 06:24:26 changed by SmileyChris

  • needs_better_patch changed.
  • stage changed from Unreviewed to Design decision needed.
  • needs_tests changed.
  • needs_docs changed.

The whole backends area is a bit messy. This is a good start to tidying it up.

I was tempted to push straight to checkin, but I'll let some more eyes look at it before then.

08/13/07 03:57:25 changed by Brian Harring <ferringb@gmail.com>

via http://groups.google.com/group/django-developers/browse_thread/thread/ab7649e4abd4e589 , might suggest getting the first patch posted into vcs; the second is following and is far more intrusive; actual api changes (no more django.db.backend access primarily), but constricts access to backend capabilities/ops/client/introspection through the wrapper object, thus allowing for far easier/intrusive hooking/override- specifically to just layer connection pooling over a backend, rather then having to write a pooler specific to each backend.

(follow-up: ↓ 5 ) 08/13/07 04:09:51 changed by anonymous

If you mean that code won't have access to django.db.backend any longer (or shouldn't be accessing it), you will want to start a design discussion on django-dev first. That's a very intrusive change and will break a lot of code, so we need to discuss the change in abstract first before wading through lots of lines of code.

08/13/07 04:13:29 changed by mtredinnick

  • owner changed from adrian to mtredinnick.

Oops... last comment was me, by the way.

I'm generally +1 on the current patch, since it looks clean. Still want to look at it once or twice more, but it'll go in soon, I would expect.

08/13/07 04:26:11 changed by Brian Harring <ferringb@gmail.com>

  • attachment backend-refactoring-v2.patch added.

version/stage 2 of the refactoring; api breaking, but molds existing old style layout into new automatically, and allows for actual OOP trickery.

(in reply to: ↑ 3 ) 08/13/07 04:44:03 changed by Brian Harring <ferringb@gmail.com>

Replying to mtredinnick:

If you mean that code won't have access to django.db.backend any longer (or shouldn't be accessing it), you will want to start a design discussion on django-dev first. That's a very intrusive change and will break a lot of code, so we need to discuss the change in abstract first before wading through lots of lines of code.

Still has access to it, just has access to a compartmentalized version of it. Already converted existing code over to it w/in django; atm, getting v2 up in the tracker (upstream for the patch is bzr branch at http://pkgcore.org/~ferringb/django/connection-pooling) prior to kicking off the discussion; it's worth noting that with a rather simple getattr, it's possible to preserve the old api (preferably firing off a warning on access).

08/13/07 08:38:27 changed by George Vilches <gav@thataddress.com>

Since we don't have an official voting system yet, just adding a comment that I'm +1 for this patch (the first or the second, I'll be happy with either), it would make the SQL logging additions much cleaner.

08/17/07 18:24:32 changed by Brian Harring <ferringb@gmail.com>

  • attachment backend-refactoring-v2-correct-bitrot.patch added.

re-cut v2, since adrians core/management horked v2 from being applied.

08/19/07 15:59:58 changed by adrian

I'm inclined to check in the first patch, as it's a simpler / more atomic change, and I'd rather not check in substantial patches like the second patch in a single commit (mostly because they're hard to follow). I'm on it.

08/19/07 16:30:57 changed by adrian

(In [5949]) Refactored all database backends to inherit from a common base class to remove quite a bit of duplicated code. Thanks for the patch, Brian Harring. Refs #5106

08/19/07 16:32:36 changed by adrian

First patch is checked into trunk. Working on the second patch now; I'm going to split it over several commits, for sanity's sake.

08/19/07 17:39:15 changed by adrian

Continued refactoring in [5950].

08/19/07 17:40:06 changed by adrian

(In [5951]) Refactored get_date_extract_sql() to DatabaseOperations?.date_extract_sql(). Refs #5106

08/19/07 17:47:44 changed by adrian

(In [5952]) Refactored get_date_trunc_sql() to DatabaseOperations?.date_trunc_sql(). Refs #5106

08/19/07 17:55:05 changed by adrian

(In [5953]) Refactored get_datetime_cast_sql() to DatabaseOperations?.datetime_cast_sql(). Refs #5106

08/19/07 18:03:38 changed by adrian

(In [5955]) Refactored get_deferrable_sql() to DatabaseOperations?.deferrable_sql(). Refs #5106

08/19/07 18:07:34 changed by adrian

(In [5956]) Refactored get_drop_foreignkey_sql() to DatabaseOperations?.drop_foreignkey_sql(). Refs #5106

08/19/07 18:13:07 changed by adrian

(In [5957]) Refactored get_fulltext_search_sql() to DatabaseOperations?.fulltext_search_sql(). Refs #5106

08/19/07 18:18:43 changed by adrian

(In [5958]) Refactored get_last_insert_id() to DatabaseOperations?.last_insert_id(). Refs #5106

08/19/07 18:25:00 changed by adrian

(In [5959]) Refactored get_limit_offset_sql() to DatabaseOperations?.limit_offset_sql(). Refs #5106

08/19/07 18:53:40 changed by adrian

(In [5960]) Refactored get_max_name_length() to DatabaseOperations?.max_name_length(). Refs #5106

08/19/07 18:59:06 changed by adrian

(In [5961]) Refactored get_pk_default_value() to DatabaseOperations?.pk_default_value(). Refs #5106

08/19/07 19:04:21 changed by adrian

(In [5962]) Refactored get_random_function_sql() to DatabaseOperations?.random_function_sql(). Refs #5106

08/19/07 19:15:53 changed by adrian

(In [5963]) Refactored get_sql_flush() to DatabaseOperations?.sql_flush(). Refs #5106

08/19/07 19:21:10 changed by adrian

(In [5964]) Refactored get_sql_sequence_reset() to DatabaseOperations?.sequence_reset_sql(). Refs #5106

08/19/07 19:24:04 changed by adrian

(In [5965]) Refactored get_start_transaction_sql() to DatabaseOperations?.start_transaction_sql(). Refs #5106

08/19/07 19:30:20 changed by adrian

(In [5966]) Refactored get_tablespace_sql() to DatabaseOperations?.tablespace_sql(). Refs #5106

08/19/07 20:03:33 changed by adrian

(In [5967]) Refactored quote_name() to DatabaseOperations?.quote_name(). Refs #5106

08/19/07 20:27:28 changed by adrian

(In [5972]) Moved postgresql backend DatabaseOperations? class into a new module, postgresql/operations.py, so that it can be imported by both the postgresql and postgresql_psycopg2 backends. Hence the two backends no longer have a duplicated DatabaseOperations? class

08/19/07 21:20:50 changed by adrian

(In [5974]) Implemented BaseDatabaseFeatures? and changed all code to access it -- connection.features.foo instead of backend.foo

08/19/07 21:21:10 changed by adrian

Note that I used "features" instead of "capabilities" because the latter just takes too long to type.

08/19/07 21:39:05 changed by adrian

(In [5976]) Refactored get_query_set_class() to DatabaseOperations?.query_set_class(). Also added BaseDatabaseFeatures?.uses_custom_queryset. Refs #5106

08/19/07 22:03:40 changed by adrian

(In [5977]) Refactored get_field_cast_sql() to DatabaseOperations?.field_cast_sql(). Refs #5106

08/19/07 22:08:32 changed by adrian

(In [5978]) Refactored get_drop_sequence() to DatabaseOperations?.drop_sequence_sql(). Refs #5106

08/19/07 22:26:55 changed by adrian

(In [5982]) Refactored OPERATOR_MAPPING so that it exists as django.db.connection.operators instead of django.db.backend.OPERATOR_MAPPING. Refs #5106

08/19/07 22:40:18 changed by adrian

  • status changed from new to closed.
  • resolution set to fixed.

OK, I think we can safely mark this as fixed. I've made 95% of the changes from the patch, with the exception of making DatabaseError, IntegrityError and the introspection and creation functionality accessible via django.db.connection object. Those changes can happen in the near future, if they need to be made. (Please open separate tickets w/ patches for those changes if you think they're worth doing.)

I ended up making small naming/implementation changes in places throughout this fix, but Brian's core design for this refactoring remained intact. Thanks for the great suggestions and patch, Brian!


Add/Change #5106 (django.db.backend.*.base refactoring)




Change Properties
Action