Re: TemplateResponse and decorator_from_middleware
> These features have the stated purpose of allowing a response to be > altered by middleware or decorators. However, if a decorator causes the > template to be rendered, then such alterations will not happen. > > This is a serious issue, because decorators like these are often applied > on an ad-hoc basis, without thinking that they are going to change > things like the body of the response. > > So, if we need to cache one page, on a site without global caching, the > obvious thing to do is to add the cache_page decorator. Unfortunately, > if you were actually using a template response middleware, you'll almost > certainly have breakage. It is really bad that adding cache_page can > change what would actually be rendered, and this could be extremely hard > to debug. Similarly with the other affected decorators. I just wanted to amend my post to say that when I said that this was a "small race condition" (is it a race condition? maybe that's the wrong term as concurrency doesn't come into play), I hadn't thought this through. Yeah, the usefulness of the TemplateResponse is much reduced when you have to worry about decorators accessing the content of the response. But that should be obvious, right, at least for caching? Certainly if you cache a response before you modify it either the cache will have to be wrong or the modifications have to will fail. -- 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.
Re: TemplateResponse and decorator_from_middleware
Hi Luke, I actually faced a problem similar to this when I ported CBV from 1.3 to 1.2, and then tried to use the TemplateResponse with a couple of middleware decorators that I created from some custom middleware classes. If I recall properly, I was also having backwards incompatibility problems with some other middleware classes that I had installed. The solution for me was actually very simple: instead of raising a ContentNotRenderedError on line 110 of django/template/response.py, I modified the code of _get_content() to call self.render(), that way making a TemplateResponse look just like a standard response with a static "content" field. Doing it this way still allows TemplateResponse to be useful, it just requires you to be careful how you order middleware classes so that the template context is updated before request.content is accessed (i.e. at the very front). It's a small race condition, but not inconsistent with the other ordering issues that go along with using middleware. That being said, I don't know if the bug I encountered was exactly the same as what you are describing, I can't quite recall the details, but I just wanted to throw this out there anyway. This may not be a complete or ideal solution, but if it works, it seems like a better option than changing the middleware API--and better than making backwards-incompatible changes to decorator_from_middleware. Regards, Ed Gutierrez -- 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.
Re: NoSQL support
> Please don't get me wrong. I have worked with RDBMS for more than a > decade but I alse use django-nonrel with MongoDB on a daily basis. I > also think that the approach django-mongokit takes is much more > natural for NoSQL data than just reusing the ORM. The ORM has no way > to express complex structures and if such support is added, you will > always have to choose which subset to use. For relational tables you'd > get foreign keys and for non-relational you'd get structure semantics. > Then we have the ModelForms that would need to start producing > sub-formsets for certain structures. In the end you end up with one > swiss army knife instead of a fork and a knife. While possible, it's > not very convenient to dine using a swiss army knife. > > -- > Patryk Zawadzki > I solve problems. You do make a good point. It's not just that NoSQL systems lack features (transactions, schema, relations) that are common to SQL- based systems; it is also the case that non-relational systems have additional features that are missing from many relational databases. Either the ORM will have to ignore those features, or it will implement them in a way that risks being convoluted and imperfect. But what is wrong with the ORM ignoring features that would risk making it convoluted? Already there are dozens, maybe hundreds of features of Oracle (including features that are NoSQL-like, like object tables and hierarchical queries) that are not implemented in the Django ORM, and that's OK. There are a number of specialized fields in Postgres that are not officially supported, some that are also NoSQL-like (array fields, for instance). Even aggregation wasn't supported at all by the ORM until version 1.1. The ORM is already making these kinds of choices in order to provide a standardized interface that can be reused without modification; anyone who needs these features can use them outside the ORM, or can extend the ORM. There is a place in the world for swiss army knives. Among the big selling points that Django has are its contrib apps and its pluggable third-party apps. The admin contrib app is a headline feature of the framework; I would call the admin a swiss army knife. There's nothing wrong with being a swiss army knife, it seems to be part of the framework's objectives. -- 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.
Re: NoSQL support
> Did you guys consider providing a Document class that is entirely > separate from models.Model? > > Technically speaking teaching the ORM non-relational tricks is of > course possible but in reality the philosophy is entirely different > and you need to plan for NoSQL from the very beginning. Traditional > models are flat and have a schema, NoSQL documents can have extra > fields and each of them can hold a fairly complicated structure, > possibly involving numerous other (python-enforced) schemas at > different points in the tree. Maybe it is inevitable that this kind of debate will crop up in any discussion of django-nonrel or NoSQL, but I very much hope that the philosophical debate does not detract from this fact: that django- nonrel has demonstrated in very real terms that the actual changes needed for Django's ORM to interface with a diverse set of non- relational systems, are, in the general scheme of things, relatively minor. Because they are localized and relatively minor, if those changes do not have a negative impact on the usability and stability of the ORM, and if they do not introduce noticeable backwards incompatibility, that small set of changes should, in my opinion, be considered for acceptance into Django. That being said... The idea that relational and non-relational systems represent entirely different philosophies is very much the conventional wisdom, but I think that when such an argument is made people often ignore the fact that object-oriented systems are just as dissimilar from relational systems as are non-relational data stores, if not more so. In fact, I think that most people would say that many so-called non-relational systems map to object-oriented systems with less "impedance mismatch" than relational systems do. The fact that django-nonrel and Alex Gaynor's GSoC project last year each took similar paths to providing non-relational functionality, arrived at independently, and that both projects required rather minimal changes to the ORM, bears this out, I think. It demonstrates that Django's ORM does not easily accommodate non-relational systems by accident, but because the generalized representation of persistent data that is an "ORM" is as good a match for many so-called NoSQL systems as it is for RDBMSs. > In the end you won't be able to move models or logic between > traditional RDBMS and NoSQL engines anyway. What we get instead is > either a whole bunch of NotImplementedErrors or a heap of hacks to > simulate traditional relations in a world that does not need them. It is also important to note that some "NoSQL" systems are more appropriate than others to be represented in an object-mapping system like Django's ORM. A redis backend, for instance, might look like a bit of a hack, but a backend for App Engine or MongoDB would implement a large and almost complete subset of the ORM functionality without any major hacks. And what is wrong with ORM backends implementing different subsets of the "full" set of ORM functionality? Already that is the case with the supported databases. Sqlite, for instance, doesn't enforce most of the constraints that the other database systems enforce. MySQL myisam does not implement transactions. Etc. Now, there has been much debate regarding hacks that are required in order to implement certain relational functionality in the ORM. The most obvious one is the question of how to handle query joins. It is important to note that simulating query joins is *not* something that django-nonrel does; query joins are simply not supported (just as they are not supported between separate databases with multi-db enabled). The modifications that django-nonrel makes are much more localized and trivial than that; many are on the order of "changing the datatype of a field from an integer to something more generic to accommodate non- integer primary keys". > Of course as much of the ORM API as it makes sense should be supported > by the Document but I really feel these should be designed as separate > object types. I think that my own personal experience might be relevant here. I use RDBMSs in by day-to-day work, and I have worked with RDBMSs in a professional capacity for over a decade. I have played around with django-nonrel quite a bit, but only experimentally. I have no "skin in the game" as it were, no professional or personal reason to want to see this integration move forward, except in this regard: having made a big professional commitment to Django, I want to see it become as popular and ubiquitous as possible, and I want to have the opportunity to use it in as many contexts as possible. Making those changes to Django trunk that would allow non-relational database adaptors to be written without patching Django would be great for Django, as a *product.*. How? It would turn the Django ORM into one of the best (certainly one of the first, if not, at this point, the only) common abstraction layers sitting atop both non-relation
Re: model fields options
> Sounds like a bad idea to me. unique_together doesn't exist on a specific > field precisely because it isn't an option for any specific fields, it's for > a set of fields. All the other options belond to specific fields. > > Alex You're probably right about this, but (while we are on the subject) aren't there some things that shouldn't be part of the model field options that currently are? Why is help_text part of the field definition? This is a ui-specific thing--what does it have to do with the database? All abstractions are leaky, sure, but this seems inappropriate. The same thing goes for editable, error_messages: these options are not part of the ORM, they are parts of the forms subsystem that have somehow ended up in the ORM. Why should the ORM know anything about forms, or any other part of Django for that matter? Eduardo -- 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.
Re: URLfield with verify_exists hangs if given an unresponsive URL (#9857)
Hi Fabian, Are there tests that isolate this functionality? If there are, I can run them against 2.5 and 2.6 to give you some independent verification. If not, you should look into how to run the Django test suite, and write some targeted tests. Regards Eduardo On May 5, 3:07 am, Fabiant7t wrote: > Hi everyone, > > verifying unresponsive URLs still hangs. There is ticket #9857, raised > two > years ago, which propagates using a timeout argument on an URLField > level. > > Unfortunately, urllib2.urlopen (which is used by URLValidator to > verify the > URL) introduced a timeout support in Python 2.6 and Django requires > 2.4+. > > For us, having the admin hang if an external ressource is unresponsive > is a > blocker and I would love to introduce an explicit timeout argument on > URLFields that Python 2.6+ respects and Python 2.4-2.5 ignores. I > wrote a > patch and attached it to the ticket:http://code.djangoproject.com/ticket/9857 > > Any thoughts? > > Best, > Fabian -- 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.
Re: EmailField max_length of 75 characters should be 256 acccording to RFC 5321
> There are plenty of other workarounds -- the easiest is to note the > email field is optional and just create a RealUserEmail object with a > OneToOne to User. Given that, a hacky workaround in Django itself just > isn't going to happen. The right thing to do is to get the auth > refactor done and solve this and all the other issues at the same > time. Might one of those "other issues" be schema migration? As the contrib apps mature, it seems that these types of problems will recur with greater frequency up until the point that South-like migrations become something that contrib apps can use. I would hate to think that every minor bug or feature enhancement such as this that affects database tables can only be fixed by a comprehensive rewrite of the subsystem :) I guess my substantive questions are these: Will the timeline of Jan Rzepecki's GSoC project permit the inclusion of "schema alteration" into Django 1.4? and: Is the "schema alteration" API that is envisioned by that project going to be full-featured enough to allow for migrations to be added to contrib apps that interact with the database? Regards, Eduardo -- 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.
Re: ModelForm validation - a better way?
> For reference, here's where we'd be in that case (I still prefer the
> context manager over the idea of two separate calls to something named
> "validate"):
>
> def my_view(request):
> form = MyModelForm(request.POST or None)
> try:
> with form.validate(tweak=True) as obj:
> obj.user = request.user
> except ValidationError:
> pass
> else:
> obj.save()
> return redirect(obj)
> return render(request, "foo.html", {"form": form})
>
> Or in the simple case, where no modifications to the object are needed:
>
> def my_view(request):
> form = MyModelForm(request.POST or None)
> try:
> obj = form.validate()
> except ValidationError:
> pass
> else:
> obj.save()
> return redirect(obj)
> return render(request, "foo.html", {"form": form})
>
> Carl
In my opinion, this looks good. I have some additional ideas as to
what the parameters to validate() should be, but in general I think
most of the most important issues are on the table. I like the idea
of having part of the validation take place in __enter__ and part in
__exit__; I think that maybe this should be controllable using the
arguments to validate() (i.e., using parameters like 'defer',
'skip_model', 'immediate'). I am also not entirely sure about using
ValidationError; perhaps the exceptions raised should be subclasses of
ValidationError, so that they can easily be caught all at once, but I
think it would be very useful for simple form validation and model
validation to raise different exceptions, primarily as a way to give
people some level of control over how errors are displayed. The
tricky nature of error generation inside model validation could be
helped a little if programmers were given the ability to distinguish
between error types by what subclass of ModelValidationError is
raised. I am also assuming that form._errors would be populated
inside validate(), __enter__() and __exit__(); it might be helpful to
create an api that makes it easier to add errors to form._errors.
The *problem* that I see is this:
The changes that we are discussing here will give model forms a
different validation API to standard forms. Yes, form.validate()
could also be added to the standard Form, but then what would the
context manager return from its __enter__ method? If form.validate()
is a method only of ModelForm, then validation of Forms and ModelForms
would be very different.
Now, in the general case, having different validation processes for
Form and ModelForm wouldn't be too much of a *practical* problem (in
most cases a programmer knows whether they are dealing with a Form or
a ModelForm). With regards to class-based views, however, and other
systems that depend on a consistent "abstract" Form API, an
inconsistent api between the two would be a headache, and possibly
limit the convenience and usability of such systems, I think. Also,
saying that validation of ModelForm works differently than Form in a
substantive way just feels wrong.
Regards,
Eduardo
--
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.
Re: "unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
Carl, Thanks for this professional reply. After rereading my post (immediately after submitting it), I realized that I was much more critical than I would normally think is fair, which is why I removed it. It's sometimes necessary, I think, to remind ourselves that most of us are volunteers here, including the core developers. I also have to say that you have made yourself available in a very generous manner of late; for that I am quite appreciative. Regards, Ed Gutierrez On Apr 28, 8:34 pm, Carl Meyer wrote: > Hi Eduardo, > > On 04/28/2011 06:36 PM, legutierr wrote: > > > This is extraordinarily discouraging. > > I can understand why. > > I've also spent a number of hours thinking about this, reviewing the > patch, considering alternatives, coming up with cases that might break, > etc. I'd like to set aside those sunk costs (which I don't think were > wasted in either case) and keep the focus on the best way to solve the > issue in Django moving forward - that's what I owe to the rest of the > core development team and to the community. > > > This is the second time that I > > have devoted tremendous energy to a patch, trying to coordinate with > > core developers, not doing any work until I get the green light from > > core developers regarding an implementation plan (trying to avoid this > > very same eventuality), only to be told, after working code + tests + > > docs have been attached to the ticket, after several iterations of > > feedback: nope, this is not the way that we want to do this policy- > > wise, there's this other approach we want to take, so never mind. > > I'm not certain what the other situation is that you're referring to, so > I can't speak to that. My observation has been that this isn't the > common experience (unfortunately, getting no attention to a bug/patch in > the first place has at times been a more common one, though that too is > getting better -- unreviewed bug count is currently zero!), but I'm > sorry you've experienced it, and I regret having contributed to it in > this case. I will certainly be more careful in the future about > expressing optimism that an approach might be workable, especially if > (as in this case) I have reservations about it from the start. > > > I can understand going through the bureaucratic rigmarole that comes > > with contributing to Django--in fact, I support it--but to go through > > all of the discussion, justification, and *time* required to get a > > simple bug fix checked in (no, this really *is* a bug--look, there are > > five other tickets filed. sure, let's analyze the problem from every > > angle. sure, I'll rewrite it so it matches exactly your > > specifications.), only to be told that someone who wasn't even > > involved in this ticket and discussion *at all* until now thinks it > > isn't worth it, makes a guy like me want to tear his hair out. You > > say that this is "in the best interests of Django", but you must know > > that Django will suffer if people like me stop wanting to contribute > > because of things like this. > > Indeed, and I hope that you don't lose interest in contributing. > > I don't think that the time spent discussing and analyzing this, even > writing and reviewing a patch, is wasted. From my perspective, it has > clearly revealed that the current approach of trying to do > partial-model-validation is broken in concept and not reliably fixable. > That's useful information, and moves us (has already moved us) towards a > better solution. > > I can't agree that this is a simple bug fix. The current behavior is > wrong in some cases. The behavior after this "fix" would still be wrong > in some cases, although fewer. A simple bug fix is one where the fix is > clear, obviously correct, and definitively solves the reported problem. > I don't think that describes this case. Model and form validation is a > complex area, and it's easy for seemingly small changes to have > unintended effects that cause more maintenance burdens down the road. > > > How often has it been the case that some really cool new feature gets > > delayed because the core developer who was working on it was unable to > > dedicate the time they thought they would be able to? Can I help move > > it along, can you work with me to write it? Why can't we check this > > one in, close two tickets (as well as satisfying three or four > > duplicates) and then move on to the more definitive fix? > > I'm committed to having these tickets closed one way or another before > Django 1.4 is released (and
Re: Suggestion for improvement to template block/extends tags
Hi amagee-
Have you tried this?
base_a.html
first {% block content %}{% block subcontent %}{% endblock %}{%
endblock%} last
base_b.html
{% extends "base_a.html %}
{% block content %}left{% block subcontent %}{% endblock %}right{%
endblock %}
template.html
{% extends (either "base.a.html" or "base_b.html") %}
{% block subcontent %}middle{% endblock %}
On Apr 28, 8:11 pm, amagee wrote:
> I sometimes run into a situation where I want a template to be able to
> extend from one of a set of possible base templates, which I achieve
> by passing a "base_template" variable in the context to the {% extends
> %} tag. Where this gets stuck, though, is if one of the possible
> bases extends one of the other possible bases.
>
> For example:
>
> base_a.html:
> first {% block content %}{% endblock %} last
>
> base_b.html
> {% extends "base_a.html" %}
> {% block content %}left {% ??? %} right{% endblock %}
>
> template.html
> {% extends (either "base.a.html" or "base_b.html") %}
> {% block content %}middle{% endblock %}
>
> I'd like to be able to code template.html so that if it extends
> base_a.html, the result is "first middle last", but if it extends
> "base_b.html", the result is "first left middle right last".
>
> I _think_ it would make sense to implement this with an improvement to
> the semantics of the {% block %} tag, starting by allowing nested tags
> with the same name.
>
> If base_b.html were:
> {% extends "base_a.html" %}
> {% block content %}left {% block content %}{% endblock %} right{%
> endblock %}
>
> Then the {% block content %} in template.html could override the
> _inner_ block in base_b.html. I think this behaviour is pretty
> logical and consistent, unless I've missed something. I've hacked
> around a bit with loader_tags.py but I'm finding it quite difficult to
> get what I want with my limited understanding of how it works.
>
> Do people think this idea makes sense? Is it worth taking the time to
> write a patch for it?
--
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.
Re: "unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
I'm up for working on the new idiom now. I've put this much time into it, I don't want to waste the momentum. What's the approach you are thinking of, and how can I get started in the implementation? On Apr 28, 4:28 pm, Carl Meyer wrote: > Hi Eduardo, > > On 04/28/2011 12:35 PM, legutierr wrote: > > > I just added a new patch in response to Carl's comments on the ticket. > > >http://code.djangoproject.com/ticket/13091 > > So, in the process of reviewing and tweaking this patch for commit, I > checked in the #django-dev IRC channel for any other core dev opinionsm > since I didn't feel 100% confident that we were doing the right thing > here. I talked through the issue with Alex Gaynor, and he successfully > convinced me that we aren't. Specifically: > > 1. We have an idea in mind, as I mentioned above, for a new > modelform-validation idiom that would solve this problem fully, by > requiring tweaks to the model to happen pre-validation and then > validating the full model. > > 2. If we implement the new idiom, and convert the admin to use it, then > anyone who runs into the problems with the current partial-validation > scheme in their own code can simply switch to the new recommended idiom. > Nobody will be stuck. > > 3. The current proposed patch on #13091 only improves the current > situation very marginally; there are a lot of cases it still wouldn't > catch (anytime a field involved in a unique-together is modified > post-validation and pre-save, and the odd exclusions for default/blank > fields). It's very much an incomplete fix, and yet it introduces new > complexity both to the code and the documentation, when we already have > a better alternative fix in mind. > > So I apologize for leading you to spend time on that patch and then > switching gears. In terms of what's best for Django, I think Alex is > right on this one, so I plan to just work on the better fix rather than > committing the current patch on #13091. > > Carl -- 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.
Re: "unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
This is extraordinarily discouraging. This is the second time that I have devoted tremendous energy to a patch, trying to coordinate with core developers, not doing any work until I get the green light from core developers regarding an implementation plan (trying to avoid this very same eventuality), only to be told, after working code + tests + docs have been attached to the ticket, after several iterations of feedback: nope, this is not the way that we want to do this policy- wise, there's this other approach we want to take, so never mind. I can understand going through the bureaucratic rigmarole that comes with contributing to Django--in fact, I support it--but to go through all of the discussion, justification, and *time* required to get a simple bug fix checked in (no, this really *is* a bug--look, there are five other tickets filed. sure, let's analyze the problem from every angle. sure, I'll rewrite it so it matches exactly your specifications.), only to be told that someone who wasn't even involved in this ticket and discussion *at all* until now thinks it isn't worth it, makes a guy like me want to tear his hair out. You say that this is "in the best interests of Django", but you must know that Django will suffer if people like me stop wanting to contribute because of things like this. I accept your apology, truly, but can I ask something else of you? Don't treat your contributors' time as something so easy to throw away. You and I (and others) have been talking about this for days. I have spent at least a day coding and analyzing. When the whole thing gets thrown away because of an IRC discussion, I don't know what to think. Carl, I asked you specifically in this very thread whether your new idiom would prevent this fix from getting checked in, before I started coding, and you answered that you "didn't think so". Should I have read that differently, more in the negative than in the affirmative? Perhaps, and I certainly will ask for clarification from you in the future. But you also gave me the green light to go ahead an write the patch in a manner that we discussed. Even if sometimes in the final event your original decision to pursue a certain path might prove not to be optimal, can I ask you to stick with it, so that people like me don't have to worry about the rug being pulled out from underneath us at the last moment? To place this in perspective: this is a *small* bug fix that adds six (6) lines of code to the file and prevents exceptions being raised to the user in the admin (and elsewhere). The "problematic" special case here is a rare edge case that, conceivably, could even be left out from the implementation. And yet, as someone who is still trying to figure out what kind of effort to put into contribute to Django, for me it is seriously discouraging for the work to be discarded. > In terms of what's best for Django, I think Alex is right on this one, so I > plan to just work on the better fix How often has it been the case that some really cool new feature gets delayed because the core developer who was working on it was unable to dedicate the time they thought they would be able to? Can I help move it along, can you work with me to write it? Why can't we check this one in, close two tickets (as well as satisfying three or four duplicates) and then move on to the more definitive fix? Regards, Eduardo PS I wrote much of this in anger, so it is rather more critical than anything I would normally post, but I think that it needs to be said. On Apr 28, 4:28 pm, Carl Meyer wrote: > Hi Eduardo, > > On 04/28/2011 12:35 PM, legutierr wrote: > > > I just added a new patch in response to Carl's comments on the ticket. > > >http://code.djangoproject.com/ticket/13091 > > So, in the process of reviewing and tweaking this patch for commit, I > checked in the #django-dev IRC channel for any other core dev opinionsm > since I didn't feel 100% confident that we were doing the right thing > here. I talked through the issue with Alex Gaynor, and he successfully > convinced me that we aren't. Specifically: > > 1. We have an idea in mind, as I mentioned above, for a new > modelform-validation idiom that would solve this problem fully, by > requiring tweaks to the model to happen pre-validation and then > validating the full model. > > 2. If we implement the new idiom, and convert the admin to use it, then > anyone who runs into the problems with the current partial-validation > scheme in their own code can simply switch to the new recommended idiom. > Nobody will be stuck. > > 3. The current proposed patch on #13091 only improves the current > situation very marginally; there are a lot of cases it still wouldn't > catch (anytime a field involved in a unique-together is modified &
Re: "unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
I just added a new patch in response to Carl's comments on the ticket. http://code.djangoproject.com/ticket/13091 Regards, Eduardo -- 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.
Re: "unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
I didn't link to where I uploded the patch. It is here: http://code.djangoproject.com/ticket/13091 On Apr 28, 1:43 am, legutierr wrote: > OK, I just uploaded a patch against trunk that should be consistent > with this discussion. As I note in the ticket, I kept the tests from > the prior patch, which tests were specifically relevant to the admin > as reported by the original ticket, but I also added additional, more > focused tests. I also had to modify one of the model_formsets tests, > as it was assuming the old behavior. > > I ran my changes against the whole of the Django test suite, and no > relevant errors seem to have been generated. > > I hope this is OK to check in :) > > Regards, > Eduardo > > On Apr 27, 3:13 pm, Carl Meyer wrote: > > > > > > > > > On 04/27/2011 02:02 PM, legutierr wrote: > > > > Ok, I'll create a patch soon (with tests + documentation) that > > > hopefully works for you. I don't think it will be very complicated > > > implementation-wise, just a few additional lines, I think. With > > > regards to the documentation, I'll add a note here: > > > >http://docs.djangoproject.com/en/1.3/ref/models/options/#unique-together > > > > and here: > > > >http://docs.djangoproject.com/en/1.3/ref/models/instances/#django.db > > > > Including a note saying that the behavior has changed > > > Great, thanks. I think this behavior change only needs to be described > > in one place (the validate_unique docs), but the text at the former link > > is actually inaccurate ever since model validation - it should be > > updated to mention that unique_together is also checked by model > > validation, with a link to the validate_unique docs. > > > Carl -- 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.
Re: "unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
OK, I just uploaded a patch against trunk that should be consistent with this discussion. As I note in the ticket, I kept the tests from the prior patch, which tests were specifically relevant to the admin as reported by the original ticket, but I also added additional, more focused tests. I also had to modify one of the model_formsets tests, as it was assuming the old behavior. I ran my changes against the whole of the Django test suite, and no relevant errors seem to have been generated. I hope this is OK to check in :) Regards, Eduardo On Apr 27, 3:13 pm, Carl Meyer wrote: > On 04/27/2011 02:02 PM, legutierr wrote: > > > Ok, I'll create a patch soon (with tests + documentation) that > > hopefully works for you. I don't think it will be very complicated > > implementation-wise, just a few additional lines, I think. With > > regards to the documentation, I'll add a note here: > > >http://docs.djangoproject.com/en/1.3/ref/models/options/#unique-together > > > and here: > > >http://docs.djangoproject.com/en/1.3/ref/models/instances/#django.db > > > Including a note saying that the behavior has changed > > Great, thanks. I think this behavior change only needs to be described > in one place (the validate_unique docs), but the text at the former link > is actually inaccurate ever since model validation - it should be > updated to mention that unique_together is also checked by model > validation, with a link to the validate_unique docs. > > Carl -- 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.
Re: NoSQL support
+ 100 on this (oh, wait, do I not get that many votes? +10 then). Waldemar and Thomas (and the rest of the people contributing to django- nonrel) have worked very hard to advance Django and expand its use into new spheres. It would be great to see their work recognized by the core team, and to begin to see the relevant parts integrated into trunk. Obviously this is only going to get done if one of the core developers has the time and desire to work with Waldemar and Thomas. As someone who uses with Django every day and has committed to the platform on a commercial basis, and as an infrequent contributor, I very much hope that someone on the core team decides to take them under their wing. Regards, Ed Gutierrez -- 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.
Re: "unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
Hi Carl- > Hmm, that's interesting. I'm not super-enthused about the complexity > there (Zen of Python: "if the implementation is hard to explain, it's a > bad idea"), but I think you're right that it's feasible. Note that > nullable fields would be ok to go ahead with (because NULL is not equal > to NULL in SQL, it won't cause false positives on the uniqueness check); > it's just fields with non-null defaults that could cause the false > positive if they are excluded from the form but included in a > unique-together check. > > If the implementation (and documentation) for that patch doesn't look > too terrible, I'd consider it - I do think it gets the behavior closer > to right than what we do now, and I'm not sure it's really possible to > get it fully right in all cases as long as we're trying to do validation > on a partial model. I'd be interested in others' thoughts, of course. Ok, I'll create a patch soon (with tests + documentation) that hopefully works for you. I don't think it will be very complicated implementation-wise, just a few additional lines, I think. With regards to the documentation, I'll add a note here: http://docs.djangoproject.com/en/1.3/ref/models/options/#unique-together and here: http://docs.djangoproject.com/en/1.3/ref/models/instances/#django.db.models.Model.validate_unique Including a note saying that the behavior has changed Regards, Eduardo -- 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.
Re: "unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
Hi Carl- > With your proposed change, if I happen to have a FavoriteBirthdayPresent > in the database for a given year with an empty "username" field, it > would incorrectly prevent any user from creating their own > FavoriteBirthdayPresent for that year. > > I'll readily admit that's a corner case that requires several > perhaps-unusual conditions: > - the excluded field that's part of a unique_together has a default > value which is also a valid value in the database, and is not None/NULL > (because NULL != NULL in SQL), and > - there actually might be a record in the database with that default value. > > And I think there are probably many more cases where your proposed > behavior would be correct. I'd just be happier marking #13091 Accepted > if I could see a solution that seemed more clearly correct in all cases. Regarding this, I have two somewhat contradictory responses: 1) It would be feasible to treat the case where a default value is defined on a field (or where the field is allowed to be null) as being distinct from the case where the default value is not defined and the field is not permitted to be null. In other words, in the case that you cite the current behavior could be maintained (unique_together tuples containing any field with a default value would be ignored in model validation), while my proposed behavior would only be implemented when model validation is certain not to create the circumstances you describe. I would be happy to write a patch for this + tests if you are OK with the approach. 2) I am not sure, however, that the case you site is really a problem. So what if the user is told that the "year" data they have supplied in such a case is not "sufficiently unique"? It would be true (would it not?) that the default "username" would already have their favorite birthday present assigned for that year (even if the default is null), and it seems to me that such a fact is intelligible to the user (The error message could read: "The data you supplied for field 'year' is not sufficiently unique for username 'default'," or perhaps simply "The 'year' you specified is not sufficiently unique."). It would also be fully within the power of the user to modify the form in order to get it to pass model validation and be saved. Maybe this is a fine distinction, but I think that saying that a field or set of fields is not "sufficiently unique" is perfectly sufficient from a usability point of view. It is a lot better than raising an exception after saving, and if the error message isn't good enough in any circumstance, it would be as easy as it is now to override it programmatically. > This is really giving me the itch to build a new context-manager-based > idiom for ModelForm validation in 1.4 that would allow modification of > the to-be-saved object within the context manager and always perform > full validation of the model on the way out. This idea was raised > briefly in discussions right around the time model-validation landed, > but was tabled due to the need (at that point) to support Python 2.4. > Seems like that could neatly resolve all these knotted issues around > validation of partial ModelForms. I am sure that whatever idiom you create will be an improvement over the current approach, but unfortunately, I think that what you are describing is a little over my head. Regardless, I hope that the prospect of introducing this new idiom will not prevent #12082 and #13091 from being resolved without the acceptance of a new approach. While we are on the subject of new idioms, I am curious as to what might be wrong with this slight amendment to the current idiom: form = ModelForm(data) form.instance.excluded_field = programatic_data if form.is_valid(): form.save() Is there some reason why programatic data needs to be assigned to the form instance *after* validation takes place? Is there something about the instance that is returned by form.save(commit=False) that distinguishes it from form.instance in such a way that it would break form.is_valid() or form.save()? Regards, Eduardo -- 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.
"unique_together" only validated in modelform.is_valid() if ALL of the referenced fields are included in the form (Ticket #13091)
The subject didn't make any sense. -- 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.
"unique_together" only validated in modelform.is_valid() if ALL of the referenced fields (Ticket #13091)
Thanks for your response, Florian. > > modelform.is_valid() fails to anticipate database integrity errors > > when those errors involve any fields that are not part of that form > > itself. > > That is wanted behaviour, eg consider my workflow: > > class SomeForm(ModelForm): > class Meta: > exclude = ['user'] > model = … > > Now in the view I do this: > if form.is_valid(): > instance = form.save(commit=False) > instance.user = request.user After reading the thread that you referenced, I agree with you that it would be *incorrect* to individually validate fields that are excluded from the form in the `is_valid()` model validation, especially in light of this common idiom (which I do use myself). I think that I described my problem too generically above; nonetheless, I do believe that there is *still* something wrong with the current implementation. I have modified the subject of this thread to reflect real source of my complaint, which is described in the last paragraph of my original ticket: > For me, this is a problem in the case of "unique_together" fields, > where one field is editable on the form, and the other is set at > record creation time or in some other programmatic way. It is > possible, even likely, that a uniqueness constraint will be violated > by a user changing the editable field, causing in the current > implementation an IntegrityError to rise to the top of the stack, > directly impacting the user. Instead, the user should be told that the > data they entered is not sufficiently unique. If I understood the gist of the model-validation thread you referenced, it is (1) that users of a form should not be presented with a validation error that they are unable to fix by modifying submission data, and (2) that developers should get directly involved if there is any error being generated at form submission time that is out of the control of the user. In other words, it is a GOOD thing for IntegrityError to be raised if instance data that is excluded from the form violates a constraint. I think that the argument is convincing, and I agree with all of these sentiments. However, in the case of a tuple of fields that are "unique together", the proper behavior should be that if *any* of those fields are editable in the form, the constraint should be validated by is_valid(). In the current implementation, *all* of the unique- together fields have to be editable for the constraint to be validated; THAT is the real bug here. It is always fully within the power of a user to resolve a "unique together" constraint violation simply by modifying any one of the fields subject to that constraint; the only thing required is that the user be told which editable field(s) are causing the constraint violation. Furthermore, such violations are not uncommon, and should not require the intervention of the developer. Note that this is not just a problem for me, but is also a problem at the root of several tickets that have already been accepted, several over a year old. These two open tickets pertain to admin and content types, and have the same root cause as I am describing above: http://code.djangoproject.com/ticket/12028 http://code.djangoproject.com/ticket/13091 also: http://code.djangoproject.com/ticket/13249 http://code.djangoproject.com/ticket/15326 Something to take note of regarding #13091, which seems to be the canonical ticket: the current patch attached to this ticket would eliminate *all* exclusions from the call to the validate_unique() model validations. This I now disagree with (as I am sure you do, too), although I originally proposed doing just that. I also think that in the case of a "unique together" tuple where *none* of the fields are editable, model validation should be skipped inside is_valid(). However, I do think that a modification is warranted to resolve the underlying error of the above-listed tickets, as well as my own. > Btw, just my humble opinion: If you exclude fields from the ModelForm > it's your duty to check for valid data. I think this is partly true. However, I believe that with respect to `unique_together`, you should relax this standard. In the case of content-types and generic fields, it is very common to exclude the content-type and object-id fields form the forms, but to also group the content-type and object-id fields with some other editable field in defining a "unique_together" constraint. In such a case, requiring a developer to write boilerplate code to validate uniqueness seems inconvenient, counterintuitive and kind of difficult, actually (IMHO, of course). Furthermore, without adding a whole bunch of complex special-case code to the admin, the current conflict between "list_editable" and "unique_together" is only solvable with the kind of change I am proposing be made to model validation. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send
Ticket #15860: modelform.is_valid() and modelform.errors fail to anticipate database integrity errors, allows exceptions to reach the user
I hope it's OK to copy the body of the ticket here, even if it is redundant, in order to spur discussion. I'm willing to work on a patch if there is a consensus as to the proper solution... modelform.is_valid() fails to anticipate database integrity errors when those errors involve any fields that are not part of that form itself. Technically, this is because the modelform.validate_unique() method uses the modelform._get_validation_exclusions() method (which lists any model fields that are not in the form itself) to define the exclusions for the call that is made to the ORM object's validate_unique() method (see here: http://code.djangoproject.com/browser/django/trunk/django/forms/models.py#L339). In practical terms this is a bad thing because, in a variety of circumstances, modelform.is_valid() returning False is the only thing that will prevent modelform.save() from being called, and modelform.save() will, in such a case, raise an IntegretyError that will not be caught. In my opinion, modelform.is_valid() should always report that a form is NOT valid if it is certain that a call to save() will raise an exception. The implementation problem here is either: 1. that modelform._get_validation_exclusions() is too liberal in its exclusions, 2. that those liberal exclusions should not be passed at all to instance.validate_unique(), or 3. that the implementation of instance.validate_unique() is using those exclusions incorrectly. It seems that the original logic was that model fields that are not part of the form should be excluded from the decision whether to mark a form as invalid. But a form *is* invalid if it cannot be saved to the database, regardless of the reason. Now, an argument can be made to the effect that model fields which are not form fields are not the concern of the form and SHOULD cause an IntegrityError to be raised, but that argument is not entirely relevant: instance.validate_unique() excludes all validations that reference *any* of the excluded fields, even if multiple-field constraints include fields that are, in fact, part of the form. So, if the user changes a field on a form that combines with another, hidden value to violate a constraint, the user will see a 404 or exception page, instead of a meaningful error message explaining how they can fix their mistake. For me, this is a problem in the case of "unique_together" fields, where one field is editable on the form, and the other is set at record creation time or in some other programmatic way. It is possible, even likely, that a uniqueness constraint will be violated by a user changing the editable field, causing in the current implementation an IntegrityError to rise to the top of the stack, directly impacting the user. Instead, the user should be told that the data they entered is not sufficiently unique. -- 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.
Ticket #15610 : Generic Foreign Keys break when used with multi-db.
http://code.djangoproject.com/ticket/15610 I just stumbled upon this unusual and problematic behavior, and thought that it might be worth a discussion. Details are in the ticket. Regards, Ed Gutierrez -- 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.
Re: django/db/models/sql/where.py: line 215
I'm afraid that I can't help you with the specific problem that you are describing, but I would like to recommend that you look at the work that Waldemar Kornewald and Thomas Wanschik are doing with their django-dbindexer: http://www.allbuttonspressed.com/blog/django/joins-for-nosql-databases-via-django-dbindexer-first-steps Their project may be solving the same basic problem you are trying to solve, but in a more generalized way, and in a way that supports more recent versions of Django. Although their primary objective is to simulate relational functionality in non-relational systems, I'm sure they would be open to seeing how their tool can make multi-db easier for relational databases. -- 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.
Re: contrib.sites and multitenancy
Carl and I had a long discussion in IRC about all the issues he raises above. I am still digesting that conversation, but there are some things that already spring to mind. 1. I can see the merits of defining a SITE_BACKEND api that would allow users to choose from different site-retrieval implementations that sites framework would contain. I wonder, would it make sense to make SITE_BACKEND the central aspect of the framework? SITE_BACKEND could be the required setting (instead of SITE_ID), and a "get_orm_site_by_id" backend, a "get_orm_site_by_request" backend, and a "get_request_site" backend could all be defined. That way, the documentation could say: "Always define a SITE_BACKEND, here are the different implementations to choose from." It would provide a good opportunity to discuss the different architectures in parallel, and would require users to explicitly choose from among mutually-explicit specific strategies, with only one setting. As it stands now with both of our proposals, documenting how all this works requires describing a very intricate and confusing conditional tree. Making the SITE_BACKEND control everything would simplify documentation significantly. 2. One difference between our proposals is that I propose that the framework's API define a *single*, unitary facade function--"get_current_site()", which takes a an optional "require_site_object" as a parameter--while Carl proposes that the framework define two facade functions--"get_current_site()", and the "Site.objects.get_current()" manager method. I see these two approaches as logically equivalent. Both proposals recognize the need to differentiate between the case where any generic site object may be returned (RequestSite, django.contrib.sites.models.Site, or some user-defined object), and the case where only django.contrib.sites.models.Site may be returned. Carl's proposal is more consistent with the Django practice of putting orm-object-retrieval logic inside managers, and avoids having to deprecate Site.objects.get_current(). My proposal more closely follows the facade design pattern, and I believe simplifies the documentation and thus simplifies things for users. I would favor my proposal, obviously, but I can understand the merits of Carl's. Also, Carl's proposal has the benefit of being what is already checked in. 3. In spite of the fact that having two facade functions does abrogate the need to define a "require_site_object" parameter in get_current_site, I think an argument can be made to include the "require_site_object" parameter in the API for SITE_BACKEND. Carl proposes verifying the object returned by SITE_BACKEND before returning from the manager method, to make sure it is in fact a Site object. If a user-defined SITE_BACKEND is coded to retrieve a custom object from the db, then an unnecessary database call can be prevented if "require_site_object" is passed in and used to return None at the top of the function. 4. I think it is important to keep in mind what the original motivation was behind #15089 and the post that started this thread: to remedy architectural inadequacies that forced the Satchmo devs, in their implementation of django-threaded-multihost, to monkey-patch SiteManager. Although using thread-locals to store the current request is something that Django frowns upon, it is the only way to achieve certain multitenancy behaviors, and seems to be required by a decent number of users (between them, django-threaded-multihost and django-multihost have 37 followers and 4 forks, and I am sure there are a number of other implementations out there). If SITE_BACKEND is called only when a request is supplied as an optional argument to Site.objects.get_current(), then the thread locals strategy employed by django-threaded-multihost et al. will still require monkey-patching SiteManager. If SITE_BACKEND is being called inside Site.objects.get_current(), it *should* be called *regardless* of whether a request is passed into it. If a particular SITE_BACKEND requires that a request be passed in for it to do properly retrieve a site, then that backend should raise an exception; it should not be that such a SITE_BACKEND is simply ignored. This is distinct from Carl's proposal, but I think not a significant deviation from it. Regards, Eduardo -- 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.
Re: contrib.sites and multitenancy
To complement the above list of locations where Site.objects.get_current is still in use, I have generated the following list of locations in the code where settings.SITE_ID continues to be in use: $ grep -nr SITE_ID * | grep -v svn | grep -v pyc conf/project_template/settings.py:39:SITE_ID = 1 contrib/admindocs/views.py:134:site_obj = Site.objects.get(pk=settings_mod.SITE_ID) contrib/admindocs/views.py:141:'site_id': settings_mod.SITE_ID, contrib/admindocs/views.py:286:site_obj = Site.objects.get(pk=settings_mod.SITE_ID) contrib/admindocs/views.py:295:'site_id': settings_mod.SITE_ID, contrib/comments/feeds.py:27:site__pk = settings.SITE_ID, contrib/comments/forms.py:155:site_id = settings.SITE_ID, contrib/comments/templatetags/comments.py:82:site__pk = settings.SITE_ID, contrib/comments/views/moderation.py:21:comment = get_object_or_404(comments.get_model(), pk=comment_id, site__pk=settings.SITE_ID) contrib/comments/views/moderation.py:47:comment = get_object_or_404(comments.get_model(), pk=comment_id, site__pk=settings.SITE_ID) contrib/comments/views/moderation.py:74:comment = get_object_or_404(comments.get_model(), pk=comment_id, site__pk=settings.SITE_ID) contrib/flatpages/templatetags/flatpages.py:22:flatpages = FlatPage.objects.filter(sites__id=settings.SITE_ID) contrib/flatpages/views.py:35:f = get_object_or_404(FlatPage, url__exact=url, sites__id__exact=settings.SITE_ID) contrib/gis/tests/__init__.py:106:# Saving original values of INSTALLED_APPS, ROOT_URLCONF, and SITE_ID. contrib/gis/tests/__init__.py:109:self.old_site_id = getattr(settings, 'SITE_ID', None) contrib/gis/tests/__init__.py:123:# SITE_ID needs to be set contrib/gis/tests/__init__.py:124:settings.SITE_ID = 1 contrib/gis/tests/__init__.py:135:settings.SITE_ID = self.old_site_id contrib/redirects/middleware.py:11:r = Redirect.objects.get(site__id__exact=settings.SITE_ID, old_path=path) contrib/redirects/middleware.py:17:r = Redirect.objects.get(site__id__exact=settings.SITE_ID, contrib/sitemaps/tests/basic.py:124: public.sites.add(settings.SITE_ID) contrib/sitemaps/tests/basic.py:131: private.sites.add(settings.SITE_ID) Binary file contrib/sites/.models.py.swp matches contrib/sites/models.py:26:Returns the current ``Site`` based on the SITE_ID in the contrib/sites/models.py:38:sid = settings.SITE_ID contrib/sites/models.py:42:"the SITE_ID setting. Create a site in your database and set " contrib/sites/models.py:43:"the SITE_ID setting to fix this error." contrib/sites/models.py:154:elif hasattr(settings, 'SITE_ID'): contrib/sites/models.py:155:return Site.objects.get_site_by_id(settings.SITE_ID) contrib/sites/tests.py:11:Site(id=settings.SITE_ID, domain="staticsite.com", name="staticsite.com").save() contrib/sites/tests.py:31:s2 = Site.objects.get(id=settings.SITE_ID) contrib/sites/tests.py:46:self.assertEqual(site.id, settings.SITE_ID) contrib/sites/tests.py:60:self.assertNotEqual(site.id, settings.SITE_ID) The reason that this list is pertinent is that it shows where the Django code is locked into a "static multitenancy" model. Unless this code is converted to use get_request_site, none of the above contrib apps will work with a "dynamic multitenancy" model. In a number of the above cases, a conversion will be trivial. However, in any case where the SITE_ID is referenced inside an object or function that is not a view, and therefore does not have reference to the incoming request, there may be a problem in eliminating the dependency. -- 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.
Re: contrib.sites and multitenancy
I have added an initial, incomplete patch to the ticket for anyone who would like to comment on it: http://code.djangoproject.com/ticket/15089 -- 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.
Re: contrib.sites and multitenancy
I have been researching changes required to implement this, and I thought I would share one of the sticky points. Some of this is relevant to Gabriel's recent post. I have been reading through revisions 14141 [1] and 14142 [2], which reversed some of the changes in revision 13980 [3], and I have discovered a couple of challenges. There are a number of places in contrib app code where the current site must be known, but where there is no request available to be passed to get_current_site() (contrib.sitemaps is the most important example, see below); in each of these cases, Site.objects.get_current() must be used. As Carl Meyer's above post so correctly described, Django as of version 1.2.4 is pretty good at "static multitenancy", but Django is very, very bad at "dynamic multitenancy" (i.e. true multitenancy). The problem is that dynamic multitenancy is determinable by the current request, whereas Django's entire multitenancy implementation is currently tied to a static global SITE_ID value, and is therefore locked into a static multitenancy mode. It is for this reason that Site.objects.get_current() needs to be deprecated. Site.objects.get_current() is 100% dependent on the global SITE_ID value, and as a result will never permit dynamic multitenancy to be implemented by any application that uses it. If the objective is to implement true multitenancy in Django using the sites framework, then the only alternative to eliminating Site.objects.get_current() in favor of get_current_site() would be to have Django store each request in thread locals, the way that Flask does [4]. In other words, in order to implement true multitenancy, either the request will have to be placed into thread locals so that it may be accessed by Site.objects.get_current(), or the request will have to be passed around to every corner of the code that needs to know about contrib.sites, so that it can be passed to get_current_site(). Now, maybe I am jumping to conclusions when I imply that using thread locals a la Flask [4] is not a viable option. Perhaps it would make sense to add a middleware that could manage thread locals data safely and provide an interface to access the stored request. It might be the easiest thing to do. But I for one do not think that it will be necessary to do so. PLEASE NOTE: the above referenced ticket does not NECESSARILY propose modifying every library that might have a dependency on Site.objects.get_current() [see below]. What this ticket is saying is that any applications that implement multitenancy in such a way that dynamic multitenancy is not an option should be modified during the 1.4 iteration so as to support dynamic/true multitenancy. It is also proposing a tangible implementation and interface that can be used to do so. The issues I am raising in this post can be delayed until future versions if need be, as long as a migration path is specified now. At least, those are my two cents. [1] http://www.google.com/url?sa=D&q=http://code.djangoproject.com/changeset/14141 [2] http://www.google.com/url?sa=D&q=http://code.djangoproject.com/changeset/14142 [3] http://www.google.com/url?sa=D&q=http://code.djangoproject.com/changeset/13980 [4] http://flask.pocoo.org/docs/design/#thread-locals The list of locations where Site.objects.get_current is still in use seems to be as follows: $ grep -nr get_current\( django/ | grep -v svn django/contrib/auth/tests/views.py:195:site = Site.objects.get_current() django/contrib/comments/feeds.py:12:self._site = Site.objects.get_current() django/contrib/comments/feeds.py:17:self._site = Site.objects.get_current() django/contrib/comments/feeds.py:22:self._site = Site.objects.get_current() django/contrib/comments/moderation.py:242:subject = '[%s] New comment posted on "%s"' % (Site.objects.get_current().name, django/contrib/sitemaps/__init__.py:33:current_site = Site.objects.get_current() django/contrib/sitemaps/__init__.py:68:site = Site.objects.get_current() django/contrib/sitemaps/__init__.py:89:current_site = Site.objects.get_current() django/contrib/sitemaps/tests/basic.py:166: Site.objects.get_current() django/contrib/sites/models.py:10:def get_current(self): django/contrib/sites/models.py:92:current_site = Site.objects.get_current() django/contrib/sites/tests.py:19:# Make sure that get_current() does not return a deleted Site object. django/contrib/sites/tests.py:20:s = Site.objects.get_current() django/contrib/sites/tests.py:28:site = Site.objects.get_current() django/contrib/sites/tests.py:33:site = Site.objects.get_current() django/views/decorators/cache.py:19:sites.get_current().domain, for example, as that is unique across a Django -- 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 uns
Re: contrib.sites and multitenancy
After having spoken briefly to Carl and Russell about in irc earlier this week, I created a new ticket that addresses the issues raised in this thread: http://code.djangoproject.com/ticket/15089 Would anyone mind accepting it and giving feedback? -- 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.
Re: #6375 -- Class Based Views: Opinions on commit plan
Thinking about it more, I think that the approach you took makes more sense. Regards, Eduardo On Oct 17, 7:49 pm, Russell Keith-Magee wrote: > On Mon, Oct 18, 2010 at 2:00 AM, legutierr wrote: > > > On Oct 17, 11:58 am, Russell Keith-Magee > > wrote: > >> On Sat, Oct 16, 2010 at 12:45 AM, Russell Keith-Magee > > >> I should also be able to port the tutorial before I commit -- which, > >> barring objection, I will do tomorrow night my time (about 24 hours > >> from now). Speak now, etc etc. > > >> Yours, > >> Russ Magee %-) > > > If it is too late for this, then just disregard, but I do have one > > slight observation about the TemplateMixin. Might it be a good idea > > to encourage alternate response mixins (JSONResponseMixin, etc.) > > implemented by the community to implement and use get_response() and > > get_context_instance() methods? If so, would it be a good idea to > > implement a BaseResponseMixin that implements those methods, as well > > as a `render_to_response` that raises NotImplementedError, that could > > be subclassed? > > > This seems like a relatively inconsequential thing, but I thought I'd > > just put it out there. Without it, I think the tendency would be for > > alternate response mixins not to contain either of these methods > > (which seem like useful hooks), or to just copy and paste what's > > there. > > I contemplated this after looking at your bitbucket fork, but decided > against it. The ResponseMixin in your branch contains three methods: > > * render_to_response() -- which must be overridden > * get_context_instance() -- which is of arguable utility in the general case > * get_response() -- which will probably need to be overridden in most > subclassing cases to provide a default content type. > > Given that the only three methods in that mixin are either not > necessary or will need to be overridden, I decided that > reimplementation would ultimately make more sense. If this were Java > and types mattered, having the common base class might make sense, but > Python is fine about ducks, so we might as well exploit that fact. > > However, you will note that the topic guide that Andrew prepared has a > section heading specifically targeted at JSON responses. That section > isn't written yet, but the idea is to put in a guide on how to handle > JSON responses as way to point out why template rendering has been > factored out in the way that it has. > > Yours, > Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@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.
Re: #6375 -- Class Based Views: Opinions on commit plan
On Oct 17, 3:51 pm, Łukasz Rekucki wrote: > > Currently, you can override only how successful responses are > rendered. I'm going to try to work on this on my branch, but I have a > small problem: In number of places, views raise Http404 which then get > rendered by the default 404 handler (which will render HTML, which is > useless for an AJAX view). I don't currently see an easy way, to > override this on a per-view basis with just a mixin. I could catch > Http404 exception, but it won't trigger middleware handling as it > normally does. Any suggestions ? > > -- > Łukasz Rekucki One option might be to catch the Http404 in dispatch(), and then call a "render_not_found" method if it is defined, and raise the exception again if it is not: def dispatch(self): try: ... return handler(response, *args, **kwargs) except Http404: if hasattr(self, "render_not_found"): return self.render_not_found(response, *args, **kwargs) else: raise render_not_found() could be optionally implemented by the render response mixin, or left out entirely for standard 404 handling. Data- oriented mixins could choose to implement it to provide a non-html error response. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@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.
Re: #6375 -- Class Based Views: Opinions on commit plan
On Oct 17, 11:58 am, Russell Keith-Magee wrote: > On Sat, Oct 16, 2010 at 12:45 AM, Russell Keith-Magee > > I should also be able to port the tutorial before I commit -- which, > barring objection, I will do tomorrow night my time (about 24 hours > from now). Speak now, etc etc. > > Yours, > Russ Magee %-) If it is too late for this, then just disregard, but I do have one slight observation about the TemplateMixin. Might it be a good idea to encourage alternate response mixins (JSONResponseMixin, etc.) implemented by the community to implement and use get_response() and get_context_instance() methods? If so, would it be a good idea to implement a BaseResponseMixin that implements those methods, as well as a `render_to_response` that raises NotImplementedError, that could be subclassed? This seems like a relatively inconsequential thing, but I thought I'd just put it out there. Without it, I think the tendency would be for alternate response mixins not to contain either of these methods (which seem like useful hooks), or to just copy and paste what's there. Regards, Ed Gutierrez -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@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.
Re: #6735 -- Class based generic views: call for comment
On Oct 5, 10:43 am, Jacob Kaplan-Moss wrote: > > * Does django.views.generic.utils.coerce_put_post() indicate a change > > that needs to be made in Django? (Is there an existing ticket for > > this?) > > Yeah, this has been a wart in Django for a while -- Django doesn't > really "get" PUT very well. Along the same lines, > request.raw_post_data is terribly named. I don't know that there's a > single ticket anywhere, but I'd like to see a general cleanup here. > > I don't see this as a blocker for class-based views, though: we have a > narrow feature deadline that I'd like to hit, and then a longer > bug-fix period we can use to clean up PUT/POST and such. I just wanted to underline what most people here already know: that according to the html 4 spec [1], the only acceptable values for form's method attribute are GET and POST. Also, in common practice, PUT is used primarily in making ajax calls, or in accessing programatic JSON or XML APIs, not in html forms. HTML5 does specify PUT as a possible value for that attribute, but if you are planning on improving your support of PUT, I have to believe that it is *primarily* because you want to improve the ability to implement programatic APIs in Django. However, as I have detailed in my last two posts in this thread, the current implementation, lacks any ajax/json/xml support. In those two posts I have detailed possible modifications to the code that would allow ajax support to be introduced. I think that Andrew Godwin's remarks contribute quite a bit to that discussion, but no one else responded yet. This is something that needs to be tangibly addressed. I am worried that with the pace that things are going here that ajax support may be ignored, or just pasted-on at the last minute. Clearly, you see this as an important feature, or you wouldn't be considering improving PUT support, and wouldn't be supporting the DELETE verb. If ajax/json/xml are to be supported by the generic view classes, however, some refactoring has to be made to the code. [1] http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.3 -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@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.
Re: #6735 -- Class based generic views: call for comment
On Oct 4, 1:04 pm, Andrew Godwin wrote: > On 04/10/10 17:28, legutierr wrote: > > > * First, treat data processing and retrieval as separable from > > rendering. Create a bright line of separation between the two > > conceptual elements of the view (data and rendering), and do it early > > on, at a high level, inside dispatch(). Instead of expecting the > > ListView.GET to return an HTTPResponse, for example, have it return a > > context or a context dictionary that could be reused with several > > render_result() implementations. > > This is problematic if I want to return something that isn't a context, > or like it (for example, if I'm rendering images, or fetching content > wholesale out of a database). What I am suggesting is not that overriding render_result would be sufficient for 100% of cases, but that it would be sufficient for 99% of cases where a standard data dictionary could be used to generate the response, whatever it may be. And while it is *conceivable* that one would want to use ListView, DetailView, etc. functionality in combination with image rendering, or other kind of unexpected content, it is not likely. However, it *is* likely that one would want to use ListView, DetailView, etc. to produce JSON, XML, PDF or other text-like content, which is the common user expectation that *needs* to be addressed. Overriding dispatch() to implement less standard functionality, or overriding get() at the same time as render_result(), would still be feasible for other cases where a simple data dictionary is insufficient for rendering. But if it is just a question of how text is being output, then custom implementation of render_result(data_dictionary) would be sufficient in 99% of cases. > So, bfirsh's previous iteration had content formats built into the base > class - I ripped it out and replaced it with a simpler base class, and > he seemed to agree, so that's where we currently are. Your simpler base class seems like a big improvement. What I'm addressing is the fact that instead of subclassing directly from the simple base class (which makes no assertion about what type of data is being returned, a very good thing), ListView, DetailView et al. subclass TemplateView. I would assert that the actual rendering logic should be implemented as a mixin, and combined with ListView, DetailView, etc. in order to produce the user-oriented generic view classes. That way, alternative rendering implementations would be much more trivial to add, and without creating a misleading class hierarchy (i.e., by having the JSONView also be a TemplateView). > My main concern is getting rid of the simplicity - e.g. of calling > render() on a template/context mix. In this aforementioned previous > iteration, if I wanted to supply custom context to a custom template, I > had to override both self.get_template_name() and self.get_context() - > even though it would have been a lot easier to override GET and just > call self.render(). It's a similar problem to passing everything around > as keyword arguments - reusability doesn't turn out to be much better > than starting from scratch. In the particular approach that I am describing, you could still override GET to modify what data is to be put into the context. And since the TemplateView render_template() implementation would use self.request to build a RequestContext, you would still be able to use context processors. The difference is that if you wanted to modify *both* what data is being added to the context and do custom stuff with the output data, you would have to override both GET() and render_result()/render_template(). Or you could just override dispatch()/as_view() in any case. > I just don't want us to build in all these abstraction layers and have > the ability to customise everything perfectly but in a verbose fashion. > That said, if render_result() in your example just calls a normal > render() method itself, it's easy to not use it if you don't have to, so > a reasonable approach to get both of these angles in is possible. render_result(data_dictionary) would call render_template(template, context) in the case of the TemplateViewMixin, just as render_to_response() does in the current implementation. However, in the case of a JSONViewMixin, render_result() would process the data dictionary to produce json output instead. I actually think that this makes things much less verbose overall. The trickier question is whether one might want to implement a MultiViewMixin, capable of outputing *both* html documents and json, for example by registering MyView.as_view and MyView.as_ajax_view in urls.py. If you look through the use cases I list below, in each of these use cases a user would benefit from being able to register two different views with two different urls using the same
Re: #6735 -- Class based generic views: call for comment
On Oct 2, 10:32 pm, Russell Keith-Magee wrote: > I completely agree that we don't want to rush this. The upside is that > if we *can* reach consensus, it isn't going to require a whole lot of > code changes; We're arguing about how to architect the base class, but > once we've got that sorted out, Ben's patch is feature complete and > shouldn't be too hard to adapt to any base class along the lines of > what we've been describing. Given that most of this thread has been dedicated to simply discussing "how to architect the base class", I think that there is an opportunity to discuss other elements of the implementation that have been neglected up to now. While bfirsh's implementation may be *almost* feature complete, it seems to be lacking one major feature that will be important to many, if not most, users; specifically, the ability to easily return data *other than* html rendered from a template. A large number of users, if not a majority, will want to be able to use class-based views to return JSON data. Others will want to render and return XML or PDF data. Some users may even want to use django-annoying's @render_to and @ajax_request decorators, and would have classy views return dictionaries instead of HTTPResponse objects. Of course, users could subclass the base View class to do any of these things, but then they would be discarding most of this module's useful functionality. The real power of class-based generic views will be found in the subclasses included in the module (ListView, DetailView, etc.). The real power will be the ability to re-use the generic list- processing, detail-processing, and form-processing code, not in the high-level architecture of the base class. Users, I think, should be shown an obvious and intuitive path by which they may reuse the data-processing logic of these generic view objects without being limited to a single output format, and without having to jump through hoops in terms of subclassing disparate objects and overriding tricky methods that differ from subclass to subclass in terms of both signature and implementation. At this moment in time, TemplateView is a base class of all of View's descendant classes in bfirsh's implementation. All inheritance flows through TemplateView for the entire library. Furthermore, every data- processing subclass of View references TemplateView's render_to_response method, which is oriented in its implementation and (more importantly) its signature only towards template processing. As a result, any user wanting to implement a JsonView, an XMLView, or a PDFView will need to subclass a subclass of TemplateView in order to reuse the generic views' data-processing code, and will also have to know quite a bit about the intricacies of how the different methods interact inside each of these classes. This sub-optimal for a number of reasons: 1. Views that neither render html nor use templates will report that they are TemplateView instances (via isinstance()). 2. In order to create a suite of generic JsonViews, for example, many different classes will have to be subclassed, and the methods that are overridden will not be consistent across those subclasses. 3. It is unlikely that any of this will be documented, and the overriding rules will not be intuitive nor obvious, leading to confusion and support requests. A moderate refactoring of the internals of the implementation, along with a willingness to see some of the lower-level implementation details of the generic view objects as part of the public API, could resolve these issues. My own suggestion is as follows: * First, treat data processing and retrieval as separable from rendering. Create a bright line of separation between the two conceptual elements of the view (data and rendering), and do it early on, at a high level, inside dispatch(). Instead of expecting the ListView.GET to return an HTTPResponse, for example, have it return a context or a context dictionary that could be reused with several render_result() implementations. * Second, have the dispatch method in View call render_result(context_dict), which will raise a NotImplemented exception in the base class, but will return in the subclass whatever the return data might be. This will be the locus of control for different data implementations, and can largely be implemented without any knowledge of the data processing details. * Third, provide different implementations of render_result() through the use of different mixins, each targeting a different output style (template, json, XML, PDF, etc.). That way, the logic that handles data processing and retrieval can be re-used regardless of what the data output may be, and vice-versa. * Finally, handle redirects, reloads, or 404's through exceptions, which would be processed inside dispatch() as well. Using exceptions for normal flow of control is generally frowned upon, but here it would allow for a simplified API description for calls to GET()
