Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leverage Django REST Framework #11

Open
cancan101 opened this issue Dec 1, 2014 · 21 comments
Open

Leverage Django REST Framework #11

cancan101 opened this issue Dec 1, 2014 · 21 comments
Labels

Comments

@cancan101
Copy link
Contributor

Given this description:

Model serializers
Because SwampDragon will be communicating with the frontend via JavaScript, and JavaScript don't understand Django models, we need to tell SwampDragon how to serialize these models into JSON (short for JavaScript Object Notation).

why not use Django REST Framework (DRF)'s serializers rather than re-inventing the wheel? Code here and description here and here. Perhaps use the renderers? They seem to solve the exact goal that you outlined.

I know you mentioned that you did not use them here: https://news.ycombinator.com/item?id=8676967:

About the serializers: You do need separate serializers from DRF (they are not the same package after all).

but you don't address the reason why.

@hagsteel
Copy link
Owner

hagsteel commented Dec 1, 2014

This is true, and it's something I have considered.
However the serializers are not the same for DRF and SD (and Django have it's own serializers and they are also different from DRF etc.).

DRF just reached version 3.0 and the serializers have been worked over quite a bit,
so if SD have a dependency on a specific version of DRF you wouldn't be able to upgrade to latest DRF.

The SD serializers are quite different from DRFs serializers as well.
They have an entry point to the object map amongst other things, and there is no depth value as it's not required by SD.
SD also don't bother with serializers for objects that aren't models (something I am considering adding though, for the sake of being able to apply validators and reformat values etc.)
This is just mentioning a few reasons why this hasn't been done.

That's not to say that one could not extend the DRF serializers, but there would be functions in the serializer that would never be used or possible have a breaking effect.
It wouldn't necessarily be an easy undertaking to do this.

This is not the first time this is mentioned however, so I see that it's something that would possibly be desired by the dev community, so I'm not knocking the idea, There is a lot more than just the serializers that have to be rewritten for this to work.

@hagsteel hagsteel closed this as completed Dec 5, 2014
@cancan101
Copy link
Contributor Author

Closed?

@hagsteel
Copy link
Owner

hagsteel commented Dec 5, 2014

The serializers are too different at the moment.

I might open a new issue to implement the DRF serializer but this wasn't really an issue, more of a question so I'm closing this for now

@hagsteel
Copy link
Owner

hagsteel commented Dec 5, 2014

The DRF serializers does a lot that SD doesn't need or do. The way it handles foreign key relationships are way different etc.

There is no depth option in SD etc. etc.
Too many differences at the moment.

LikeI said, I'm not knocking the idea of implementing them, but it becomes a dependency.
What about version changes to DRF etc?

Most people would probably be okay with being tied to a version of tornado-redis but not DRF.

EDIT: If someone wants to create a pull request for this I will of course review it.

@cancan101
Copy link
Contributor Author

You should loop in @tomchristie but I think now that DRF is on v3, the API for Serializers should be relatively stable.

@hagsteel
Copy link
Owner

hagsteel commented Jan 9, 2015

It would be nice if DRF serializers could be used.

I think a solution where the existing serializers and DRF serializers can both work would be ideal.

If I rewrote the way serializers are handled this could be a possibility (even go as far as making serializers swappable).

Whatever solution I (or someone else) come up with, should not create a direct dependency on DRF 3.x
and it should be backwards compatible with the current version of SD.

I am considering this though.
The SD serializers are not without their shortcomings and there are things I want to change with them so I might plan out an update and see if I can include a way to use the DRF serializers as well.

@tomchristie
Copy link

More than happy to help out with any questions if helpful.

I wouldn't be too concerned about tying yourself to API stability with REST framework - the 3.0 upgrade was something of an exception - for medium point releases (ie 3.x - which is what we'll be having for the foreseeable future) we have a strong deprecation policy that's essentially in line with what Django does.

http://www.django-rest-framework.org/topics/release-notes/#deprecation-policy

Also I can't see the serializers changing substantially at this point.

@hagsteel hagsteel reopened this Jan 9, 2015
@hagsteel
Copy link
Owner

hagsteel commented Jan 9, 2015

This might be doable with an adapter between the serializers and some work done to the current SD serializer to fit all this in.

I will ponder this over the weekend.
I think file upload might be an issue, but not sure yet.

@pcompassion
Copy link

+1!

@hagsteel
Copy link
Owner

Quick update on this:
TL;DR: It will be done but it's going to take some considerable time

I've been poking at this for a bit now and the serializers are very different indeed, and this is going to be quite an arduous task.

At this point I'm more inclined to rewrite both the serializers and route handlers which is not going to be a small task.

The DRF serializers are (of course) not aware of the data maps.

What this actually means:

The data map contains information on how objects should be mapped.

class Parent(models.Model):
    name = models.CharField(max_length=100)


class Child(models.Model):
    name = models.CharField(max_length=100)
    parent = models.ForeignKey(Parent)

If you have a serializer for Parent including children, when I make a change to a Child it will contain information for the data mapper to know which parent it belongs to (rather than serializing EVERYTHING every time, this means a small message can be sent to the client with just the updated fields and the mapping data)

This doesn't only affect the route handler but the self publishing models as well.

In short: I'm happy to do this, but the work it involves is not tiny so it's going to take quite some time to do this (fitting this around haivng a baby and my day job).

@argaen argaen mentioned this issue Feb 1, 2015
@hagsteel hagsteel added the 0.4.3 label May 6, 2015
@hagsteel
Copy link
Owner

hagsteel commented May 8, 2015

This is now available in the 0.4.3 branch (it's experimental at the moment)

@silentninja
Copy link

Was waiting for this update!!!!.Really excited to test it out

@chogarcia
Copy link

+1!

@hagsteel
Copy link
Owner

If anyone has tried the 0.4.3 branch I would love some feedback

@sylflo
Copy link

sylflo commented May 29, 2015

I'm trying to do the first tutorial using DRF. And after something a little more complex.
In DRF you can use a model to create a serializer. But SwampDragon needs a serializer in its model.

I've got serializers.py

from rest_framework import serializers
from .models import TodoList, TodoItem

class TodoListSerializer(serializers.ModelSerializer):
    class Meta:
        model = TodoList
        fields = ('id', 'name', 'description')
        publish_fields = ('name', 'description')


class TodoItemSerializer(serializers.ModelSerializer):
    class Meta:
        model = TodoItem
        fields = ('id', 'done', 'text')
        publish_fields = ('done', 'text')
        update_fields = ('done', )

And models.py

class TodoList(SelfPublishModel, models.Model):
    serializer_class = TodoListSerializer
    name = models.CharField(max_length=100)
    description = models.TextField()


class TodoItem(SelfPublishModel, models.Model):
    serializer_class = TodoItemSerializer
    todo_list = models.ForeignKey(TodoList)
    done = models.BooleanField(default=False)
    text = models.CharField(max_length=100)

And I got

ImportError: cannot import name TodoList

I don't know how to resolve this issue.
I want to use model = in the serializer file if possible.

@silentninja
Copy link

@sylflo Please create a new issue.
Now answering the question, its a circular import error.

Solution:Use a string of the model name in the metaclass of the serializer instead of importing the actual model.

@pcompassion
Copy link

Would it be possible to define multiple serializers for one model class?

SwampDragon needs a serializer in its model. makes me ask the question.

@hagsteel
Copy link
Owner

You can create several serializers for one model but you can not have multiple serializers on a selfpublish model. However I am currently unsure of the future of the self publishing model. People don't seem to be able to use it properly and get upset when it's either creating too many queries or not enough information so at the time of writing this I would say don't use the self publish model and let the routers handle the publishing

Also, this doesn't really have anything to do with the DRF serializers so let's not continue this conversation here.

@pcompassion
Copy link

@jonashagstedt Thanks for the quick answer. I'll try the tutorial and will be back to you asap.

@jsenecal
Copy link

@jonashagstedt do you have any updates on this? I am myself a strong DRF user and just stumbled across your project (it looks awesome btw) but would like to avoid having to write another set of serializers.

@hagsteel
Copy link
Owner

@jsenecal You could try the 0.4.3 branch.
I would love some help with feedback on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants