On Sat, 2007-10-06 at 18:49 +0200, David Larlet wrote: [..] > I use django-rest-interface too and I have this bug which need to be > quickly fixed so I submitted a patch which need review on #5682, let > me know if I need to do something else. I will add a patch for > modpython too when this one will be accepted.
I had a quick read of it. Looks like you're on the right track. The main problem I see with it is that you're over-populating the data structures a bit. If the request is a PUT, you're still populating self.POST as a side-effect. We should neaten this up a bit. When I started to think about this and how we would use it, it's not a complete no-brainer and I'm beginning to think my original suggestion (and what Andreas implemented in django-rest-interface) might not be the easiest for end users (third-party programmers like yourselves). A couple of possibilities spring to mind: (1) We always populate self.POST and only self.POST. Since the recommended way to check the method is to check request.method first, it means the same code can potentially be used to handle PUT and POST data. (2) We populate self.POST if request.method == 'POST' and self.PUT if request.method == 'PUT' and leave the other one blank. (3) We leave self.POST available for backwards compatibilty, but use a more generic name like self.DATA for the data we extract. This is a version of (1) with renaming involved. Right at the moment, I'm probably slightly in favour of option (1) because it's the smallest change and we can think of self.POST as "the posted data" regardless of how the HTTP verb is spelt. Code will act based on request.method, but there are more possibilities for factoring out common pieces when working with the data. So then the only change that is needed is to also populate that field when the method is "PUT" (with accompanying changes in HttpRequest). I'd be interested in hearing your thoughts, Peter and David, on this, since you've already got experience using that piece of the framework. Is there a lot of commonality between the way you're handling the PUT and POST paths? I know it's a slight change to Andreas' code, but it's not too intrusive from the looks of it. [Andreas: have I forgotten some subtlety here?] My mental example when I'm thinking about this is always how would something like Atom Publishing work, but that doesn't make me lean one way or the other here. So more examples might help. > Peter, you can test it too, it solves the bug with django-rest-interface. > > Two questions: > * did I need to write tests for this new verb? Ideally, yes. It will probably require some changes to django.test.client to allow the news type of request. Then some additions to tests/modeltests/test_client would be appreciated. > * did I have to do something in django.http.HttpRequest? Yes. Search for wherever POST is referenced there and you'll need something similar for PUT. __getitem__ and _set_encoding are the two obvious places, although the latter one might be made moot if we put all the data into request.POST regardless of the verb. In that case, though, we'll need to make some of the errors and the __repr__ formats clearer, since referring unambiguously to "POST" may be confusing. I understand that you guys are eager to get something done here so you can continue to track Django's trunk, so I'll try to respond quickly in this thread. But right at the moment, the design is as good as I think it can be, so let's take a moment to work on that. We're pretty close, though. Regards, Malcolm --~--~---------~--~----~------------~-------~--~----~ 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 [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~----------~----~----~----~------~----~------~--~---
