On Mon, Dec 15, 2014 at 9:14 AM, Fred Stluka <f...@bristle.com> wrote:
> Collin and Russell (and anyone else), > > Do you have any opinion on this? > - https://bitbucket.org/aptivate/django-current-user > > It was offered in an earlier post: > - https://groups.google.com/d/msg/django-users/y7aIbN2_CsA/GtmrSjG1nq8J > > as a solution to exactly this problem. > > Makes the current user available to the save() method of all > models. > > I read the code and it looks good, but I don't know the details > of Apache/WSGI/Django/Python multi-threading well enough > to know if it is thread safe. > I haven't deeply audited the code, but based on the approach, I can't think of any reason it wouldn't be. A signal is just a mechanism for registering pieces of code to run at known points in object lifecycle (in this case, just before the internal mechanics of save() are executed). There's nothing here that should trigger any multithreaded or cross-process concerns AFAICT. These signals aren't fired in a separate thread - it's completely linear code execution, via a code path that has been injected via signal registration. > Thoughts? > IMHO: It's much better than a thread local, but my own engineering taste says I'd still prefer an explicit call. I think the current-user package is a fine example of the possibilities afforded by signals. The problem is that I don't like signals. I've been burned by signal based frameworks in the past - if you lean on signals as an engineering approach, it's *really* easy to get into an exponential signal explosion (i.e., one signal causes 2 signals to be emitted, which causes the first signal to be emitted again, and so on). Signals are sometimes unavoidable, but as a matter of taste, I avoid them if at all possible. I vastly prefer an explicit argument, for two reasons. 1) It's very clear that it's being used, and where the value for the argument is coming from. If you use the signal approach, and I'm reading your code, and I don't know that you're using the CurrentUser signal approach, then it will be completely opaque to me why and how the current user is being specified - and there's no immediately obvious source of places to start digging. I have to audit the entire codebase to find the signal that is doing the job. This *could* be addressed with good project on-boarding documentation... but when was the last time you got given a complete and correct on-boarding document when joining an existing project :-) 2) The signal approach won't (necessarily) work in all situations - an explicit argument will. For example, if you want something on a form to be pre-populated by the current user, there isn't a signal waiting to be used. Best case, you'll have to define your own "pre-form-render" signal - which is certainly possible, but it isn't something that is waiting to be used out of the box, and you'll need to modify the Middleware to honour that signal as well. This means your middleware will need to have a signal for every possible way that a current user might be used, and signals aren't free - a signal with no listeners still has a processing overhead. Essentially, this means that Current-User *can't* be both high-performance, *and* applicable to every situation. You either need to customise it for every project where you use it, or wear a non-trivial overhead for supporting signals you may never use. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django users" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-users+unsubscr...@googlegroups.com. To post to this group, send email to django-users@googlegroups.com. Visit this group at http://groups.google.com/group/django-users. To view this discussion on the web visit https://groups.google.com/d/msgid/django-users/CAJxq84-hiAc-8rXw%3DihDg%2BM-f6ABbuLP8NzjJbA4v5TgoTLBgg%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
