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

Resolve resource type in serializer's meta class #1019

Open
sliverc opened this issue Nov 30, 2021 Discussed in #1017 · 8 comments
Open

Resolve resource type in serializer's meta class #1019

sliverc opened this issue Nov 30, 2021 Discussed in #1017 · 8 comments
Labels

Comments

@sliverc
Copy link
Member

sliverc commented Nov 30, 2021

utils.get_resource_type_from_serializer is called several times throughout the DJA code base. When this method raises an AttributeError it is a configuration/code setup issue and not a runtime issue. Besides there is no error message describing want went wrong.

Goals:

  • move logic of get_resource_type_from_serializer to serializer's meta class
  • improve error handling (better error message).
@sliverc sliverc added the bug label Nov 30, 2021
@sliverc
Copy link
Member Author

sliverc commented Nov 30, 2021

See discussion #1017 for more details.

@jokiefer
Copy link
Contributor

jokiefer commented Dec 1, 2021

i experiment with the meta class implementation at my fork

In principle that works - but i think we should rethink about the goal of moving the logic to the meta class creation, cause all serializers are configured on django start and at this point many tests have to become refactored. Maybe i can do that refactoring, but it will take a while.

The easier way would be just to add the detail information on the AttributeError.

@sliverc
Copy link
Member Author

sliverc commented Dec 2, 2021

In terms of API moving the logic of calculating relation resource type. I understand that this is not a easy and simple task. So for a quick fix I am happy to receive a PR which adds details to the AttributeError. We can then leave this issue open for further refactoring later on.

@jokiefer
Copy link
Contributor

jokiefer commented Dec 2, 2021

Ok - i'll do it quickly.

Is there any program workflow cheatsheet? I would do a docs enhancement in a separated issue if not.

@sliverc
Copy link
Member Author

sliverc commented Dec 2, 2021

As DJA is a DRF extension the worklow is the same. Not sure whether there is a sketch like this for DRF but I guess it would make most sense to add to the upstream Django REST framework doc.

Feel free to sketch something though and open a discussion about it.

@SafaAlfulaij
Copy link
Contributor

While we are at this, what do you think if we allowed serializers themselves to define what kind of type they are, via a function?
This would overwrite the attribute access.
Something very similar to what DRF does with queryset/get_queryset and serializer_class/get_serializer_class in views.
This would help for polymorphic relations, generic relations and custom non-ORM relations and serializers.

@jokiefer
Copy link
Contributor

I think this would match the django common way. Many django porjects does it like that.

@SafaAlfulaij
Copy link
Contributor

I'm also thinking if we make included_serializers and included_resources the same way. This also would help polymorphic relations, generic relations and custom non-ORM relations and serializers.

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

3 participants