Hi Brian,
Just a quick question from me regarding your comment about supporting
multiple databases... I've had a quick look through your patch and it seems
to cover a lot of the ground that the multi-db has already changed.
- I was wondering if you're familiar at all with mulitple-db branch
yet? (You sent me an email about getting the patch for it a little while
ago, and just wondered if you'd looked at the existing code).
- If you are do you have any idea how much your patch will impact the
existing work?
- How do you see supporting multiple databases progressing?
I've been really busy lately, but if you think it might help, I can update
the branch with the latest changes from trunk and either fire you a patch or
try and get the mirrored SVK repo somewhere public...
Cheers,
Ben
On 15/08/07, Brian Harring <[EMAIL PROTECTED]> wrote:
>
> On Tue, Aug 14, 2007 at 04:36:22PM -0500, Adrian Holovaty wrote:
> > After a cursory read-through the patch, my only major complaint is the
> > unnecessary cleverness of some of the code.
>
> Alt example besides BackendOps/BackendCapabilities would be
> appreciated (your phrasing implies there is more); those two classes
> are frankly borderline frankenstein at the moment due to continual
> mangling.
>
>
> > The stuff in BackendOps,
> > for example, could be rewritten in a much clearer way. (Granted, it
> > would be longer, but that's an acceptable tradeoff, in my opinion.)
> >
> > For example, this is in the patch:
> >
> > class BackendOps(object):
> > required_methods = ["%s_sql" % x for x in
> > ('autoinc', 'date_extract', 'date_trunc', 'datetime_cast',
> > 'deferrable', 'drop_foreignkey', 'fulltext_search',
> 'limit_offset',
> > 'random_function', 'start_transaction', 'tablespace')]
> > required_methods += ['last_insert_id', 'max_name_length',
> > 'pk_default_value', 'sequence_name', 'get_trigger_name']
> > required_methods = ["get_%s" % x for x in required_methods]
> > required_methods = tuple(required_methods)
> >
> > And this is a much, much clearer way of expressing exactly the same
> thing:
> >
> > class BackendOps(object):
> > required_methods = (
> > 'get_autoinc_sql',
> > 'get_date_extract_sql',
> > 'get_date_trunc_sql',
> > 'get_datetime_cast_sql',
> > 'get_deferrable_sql',
> > 'get_drop_foreignkey_sql',
> > 'get_fulltext_search_sql',
> > 'get_limit_offset_sql',
> > 'get_random_function_sql',
> > 'get_start_transaction_sql',
> > 'get_tablespace_sql',
> > 'get_last_insert_id',
> > 'get_max_name_length',
> > 'get_pk_default_value',
> > 'get_sequence_name',
> > 'get_get_trigger_name'
> > )
> >
> > Not only is it clearer, but it saves us from having to do calculations
> > at run time.
>
> While that may skip a calculation, honestly surprised there is no
> complaint about 2-10 lines below where it's shoving a curried func
> into locals (half expected a knee jerk "yuck" on that one ;).
>
> As indicated further up, those two classes will need another cleanup;
> the currying/locals I'd advocate leaving in, but expanding
> required_methods there is definitely doable- it's form right now is
> just a left over from how I went about pinning down what ops were
> 'standard' for backends. Crap like that will be removed next time I
> do a full walk of the changes (this round is basically a "this is the
> api I'm proposing, and here is a working patch for the api").
>
> So... any spots that don't read as just idiocy? Specific complaints
> about the presented API, seperation, etc, is what I'd like to get if
> possible- still have a lot of gutting to do internally anyways, so the
> form of it ought to improve.
>
> ~harring
>
>
--
Regards,
Ben Ford
[EMAIL PROTECTED]
+628111880346
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
