Opinions please...

165 views
Skip to first unread message

Tom Christie

unread,
Nov 4, 2012, 10:40:41 AM11/4/12
to django-res...@googlegroups.com
The last few days have been really helpful for getting a feel of how 2.0 actually stands up to real world usage, I'd I've been trying to concentrate on tightening up some of the potentially difficult areas, specifically the serialization and related fields.

One thing I've noticed quite quickly is that I tend to find myself having to stop and think before I write a bit of code that initializes a Serializer.

To recap from the docs, that'll typically look something like this:

  serializer = AlbumSerializer(instance=album)  #  retrieve
  serializer = AlbumSerializer(instance=queryset)  #  list
  serializer = AlbumSerializer(request.DATA, instance=album)  #  update
  serializer = AlbumSerializer(request.DATA)  #  create

I made the choice about the ordering of the 'data' and 'instance' arguments the way I did because it mirrors the Forms API - `instance` is actually pretty much the same as `initial`.

In actual usage, this doesn't feel totally natural to me, and I've been thinking that it might not be quite right.
To my eyes at least, the following feels much more natural.

  serializer = AlbumSerializer(album)  # retrieve
  serializer = AlbumSerializer(queryset)  # list
  serializer = AlbumSerializer(album, data=request.DATA)  # update
  serializer = AlbumSerializer(data=request.DATA)  # create

It's a little different from the Forms API, but then the usage and context *is* a little different.  With serializers, the common case is one of the first two, 
many APIs won't even use the update or create cases anywhere.  Conversely Forms are used exclusively for presenting instances when creation or editing *is* a possibility.

In any case, the upshot is that the later set of operations feel much nicer to me, and also mean that the slightly horrible 'instance' keyword argument (which may in fact be an instance or iterable of instances) is no longer present.  The problem of course is that there's no way to make this change without introducing a big backwards incompatible change.

I'm wondering, given how recently released 2.0 is, if I can get away with making this change (with an accompanying big bold warning and explanation in the release notes) on the basis that it'll be worth it in the long run.  (I'm hoping that 2.x is going to be with us for a long time)

I also imagine that quite a few folks will simply be using the default generic views, so this wouldn't affect them.

Still, I need the outside perspective.  What do you folks think - is this a change worth making and is it okay to be breaking compatibility in this way for the sake of future usability.  (Of course I'm slightly kicking myself for not considering this more fully prior to launching 2.0, but things were pretty manic.)

Cheers,

  Tom

Marc Tamlyn

unread,
Nov 4, 2012, 12:22:22 PM11/4/12
to django-res...@googlegroups.com
I agree solely on the point that AlbumSerializer(album) not working is mental. I actually find the fact you can use the init method of a Serializer to deserialize data a little strange but I guess it makes sense. (Disclaimer - have not written any code with DRF2 yet.)

Marc

Tomi

unread,
Nov 4, 2012, 1:47:27 PM11/4/12
to django-res...@googlegroups.com
On 04.11.2012 17:40, Tom Christie wrote:
> serializer = AlbumSerializer(album) # retrieve
> serializer = AlbumSerializer(queryset) # list
> serializer = AlbumSerializer(album, data=request.DATA) # update
> serializer = AlbumSerializer(data=request.DATA) # create

I think this feels more natural than the current implementation.
Breaking the backwards incompatible is not desirable thing, but as 2.0
was published only recently I don't think there are many apps which uses
2.0 on production already. For those people breaking the API is annoying
thing but for the rest of us it would be a welcome modification IMO.

--
Tomi

Kit La Touche

unread,
Nov 4, 2012, 2:48:43 PM11/4/12
to django-res...@googlegroups.com
I agree with what seems to be the clear consensus: the signature you're describing is better than the current one.

The question is really whether breaking compatibility on a point release is acceptable.

I think that, given that this has been out for mere days, the risk in breaking compatibility is minuscule. My one remaining concern is whether someone in the future might be confused by a mismatch between the 2.0 documentation and 2.0.1, or whatever. That seems, frankly, like a very slim chance.

I say go for it.

--Kit

Francis Devereux

unread,
Nov 4, 2012, 6:45:57 PM11/4/12
to django-res...@googlegroups.com
On Sunday, 4 November 2012 19:48:45 UTC, Kit La Touche wrote:
I agree with what seems to be the clear consensus: the signature you're describing is better than the current one.

The question is really whether breaking compatibility on a point release is acceptable.

I think that, given that this has been out for mere days, the risk in breaking compatibility is minuscule. My one remaining concern is whether someone in the future might be confused by a mismatch between the 2.0 documentation and 2.0.1, or whatever. That seems, frankly, like a very slim chance.

If the version with the change in it is 2.1.0 instead of 2.0.1 then I think people will be less surprised by a backwards-incompatible change. If you were following http://semver.org/ then you'd have to bump it to 3.0.0, but I think that's going a bit far.

(BTW I'm not close enough to the change itself to really have an opinion on it, just thought I'd chime in on version numbering).

Francis

Tom Christie

unread,
Nov 5, 2012, 11:53:21 AM11/5/12
to django-res...@googlegroups.com
Okay, it is done. 2.1.0 is released.
Reply all
Reply to author
Forward
0 new messages