On Thu, Jul 30, 2009 at 7:16 PM, Russell Keith-Magee<freakboy3...@gmail.com> wrote: > > On Fri, Jul 31, 2009 at 7:01 AM, Alex Gaynor<alex.gay...@gmail.com> wrote: >> >> On Thu, Jul 30, 2009 at 8:58 AM, Dj Gilcrease<digitalx...@gmail.com> wrote: >>> >>> On Thu, Jul 30, 2009 at 5:55 AM, Russell >>> Keith-Magee<freakboy3...@gmail.com> wrote: >>>> Of course, Alex and I may have missed an obvious alternative solution >>>> - or a use case that isn't solved by either approach. Any feedback or >>>> alternative approaches we may have missed are most welcome. >>> >> >> Russ: My primary concern with adding a new method is that it will >> signficantly complicate the implementation, particularly it pollutes >> the implementation, rather than putting the onus on calling code, >> which I've found makes it a very clean transition. In total there are >> under a dozen places within Django that call these methods, and >> eventual deprecation/removal would be fairly easy. I haven't actually >> found a usecase for user code calling into get_db_prep_* or db_type. >> I should also note that I've found a total of 2-3 instances of user >> code callign these methods on Django field classes in all of google >> code search, indeed far fewer than the number of places these methods >> are implemented. So I do still believe that my original proposal is >> the least invasive, and reasonably backwards compatible (it's worth >> remembering just be the nature of this discussing we're already >> talking about a veyr minority of users who have custom field classes >> with these methods). > > I've said this before Alex - but at some point in your youth were you > assaulted by an insane english teacher with a dislike of paragraphs? > :-) Seriously - a line break or two can do wonders for clarifying the > organization of your thoughts. > > Regarding it being a "clean transition" - clean for who? > > Lets put this to James' Documentation Driven Development test. Write > the release note for your approach: "get_db_... now takes connection, > but don't call it directly, use this other method for the next two > releases (after which you _will_ call the function directly)..... > Watch out for errors raised when using code calling the old methods > when the backend being used doesnt match the default.". >
My release notes would look like: The get_db_prep_* and db_type methods of Field classes have been updated to take a connection parameter. If you have any of these methods defined in custom field classes they should be updated to take a connection parameter (they will continue to work without for the next 2 releases). There's no need to document the calling convention for it because people will know if methods within their own code don't take the argument. > Now write the release note for my approach: "Use new_get_db_.... Old > uses will continue to work unless connections aren't compatible". > > Which one is going to be easier to explain as a transition policy? > > Consider another example: Which one is going to be easier to diagnose > when this question comes to django-users: "My custom field was working > fine, but now I'm using a second DB connection and it doesn't work? > Why?" > Do you have any custom Field classes defined anywhere that use the global connection object? Indeed the issue here isn't that they don't take the argument, it's that they use the global connection object, which will be an issue *anywhere* in their code. > My answer: "Have you implemented new_get_db_*? Are you invoking the > new function?" > > Your answer: "Have you got a get_db_ method that accepts connection as > a second argument, but with the default connection as the default > value? Are you using the transition function to invoke the function > instead of calling it directly?" (and note that this answer changes 18 > months later when the deprecation plan for the transition function > kicks in). > > As for a new method "significantly complicating the implementation" - > you're saying that a handful of new methods that defer to the original > when appropriate is _more_ complicated than a wrapper function that > uses introspection to determine the parameter selection that will be > used to the implementation? Seriously? Clearly you've had a much > better experience with introspection than I have. :-) Well the function that doesn't the introspection is about 4 lines :) > > Lastly, your comments don't address two of my concerns: > - Not raising an error when it is known that the original function won't > work. > - The normal path being zero impedance. > > The second point _might_ be addressed by the exception-catching > approach I mentioned, but you given any comment on that approach. > > Yours, > Russ Magee %-) > > > > Jacob's suggestion of a metaclass, *very* slightly appeals to me. It's what we did for the maxlength to max_length transition after all :) I'd have to think about it a little more, but it would certainly work and only have overhead at class compilation. Plus we don't even need the class attribute as Jacob suggests, we can just do some introspection there :) Alex -- "I disapprove of what you say, but I will defend to the death your right to say it." -- Voltaire "The people's good is the highest law." -- Cicero "Code can always be simpler than you think, but never as simple as you want" -- Me --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~----------~----~----~----~------~----~------~--~---
