vitalik / django-ninja

💨 Fast, Async-ready, Openapi, type hints based framework for building APIs
https://django-ninja.dev
MIT License
6.98k stars 420 forks source link

Merge some of the batteries from `django-ninja-extra` #725

Open baseplate-admin opened 1 year ago

baseplate-admin commented 1 year ago

Is your feature request related to a problem? Please describe. I'm always frustrated when there is not enough batteries available in the django-ninja codebase.

Describe the solution you'd like I want to merge some of the modules offered by django-ninja-extra into the main django-ninja repo.

Some of them are :

To be honest i dont think ninja is a bad package. But i think it is lacking some necessary features. takahe dropped ninja due to its lack of features ( or so i get from following their development because they had to rewrite pagination module ) and at this rate most new projects will drop ninja.

Theres also this from a django maintainer. ( can we address this issue? )

Alternatives I have considered Maintaining an upstream fork that has more features ( not a fan cause this will divide the user base further )

Solutions I would like Address https://github.com/vitalik/django-ninja/discussions/396 and give some talented people the opportunity to make this module better

cc @eadwinCode

minusf commented 1 year ago

interesting because a first look at hatchway seems quite barebones compared to ninja. no routes, namespaces, versioning, dynamic schema generation, etc.

baseplate-admin commented 1 year ago

hatchway will probably have more feature as tjme goes on..

vitalik commented 1 year ago

interesting because a first look at hatchway seems quite barebones compared to ninja. no routes, namespaces, versioning, dynamic schema generation, etc.

exactly how django-ninja started :D

as for the topic - well async pagination is good candidate - PRs welcome

permissions, caching - this will be reused from core-django (I'm working on a special decorator that will expose actual view function decoration)

throttling not sure yet - as of my questionnaires companies mostly use global throttlers on Middleware layer

baseplate-admin commented 1 year ago

throttling not sure yet - as of my questionnaires companies mostly use global throttlers on Middleware layer

This can be offloaded to django-ratelimit.

Consequently would you mind seeing #397 and https://github.com/vitalik/django-ninja/issues/684#issuecomment-1493218800 ? I would like your opinion on that matter

KyeRussell commented 1 year ago

This issue feels a tad negative and misrepresentative.

I appreciate the considered approach to Django Ninja's development. A well-considered API design that's built to last is exactly what the community needs, given the current choices for building API-driven Django projects. Not to knock whoever is behind django-ninja-extra, but from even a quick glance it is obvious the package exists to add things that aren't proven enough to add to Django Ninja. Using it instead of Django Ninja for anything 'production-grade' is a questionable decision. It's not something I'd do myself.

takahe dropped ninja due to its lack of features

@andrewgodwin wrote Hatchway as a result of an oddity of the Mastodon API. What Andrew needed from Django Ninja is very clearly and by his own admission niche. I can't say that I've ever had a need for it. Hatchway is not stable and it puts an outsized amount of attention on scratching Andrew's itch. Which is completely fine and understandable! But it's not eating Django Ninja's lunch.

and at this rate most new projects will drop ninja.

This is just an anecdote, but the only thing that django-ninja-extra claims to address that I'd personally want and that'd take a material amount of time to build in-house is the permissions systems. However it appears to be largely lifted from Django REST Framework, with an API design that definitely makes that apparent. I wouldn't want to see it implemented in Django Ninja in this way. The rest of its functionality...I could honestly take it or leave it.

The notion that this matters in the first place is representative of the 'treat open-source packages as business or personal vanity project' mentality that was popularised by the JavaScript community. It's not something that we should not try to replicate here. You don't build good software by sending user requests straight to the backlog.

Theres also this from a django maintainer. ( can we address this issue? )

Carlton is a past maintainer of Django REST Framework. The discussion you linked pertains to Django REST Framework. You created it! I'm not sure how Carlton's contributions to Django relate.

Router resolution depending on load order is certainly a drawback but it realistically isn't going to affect the vast majority of uses. Especially as it is partially mitigated by using Routers, which'd be the vast majority of use-cases where this would matter in the first place.

Most importantly though, django-ninja-extra wouldn't address any of the concerns that you've raised. It does not address the use case that Hatchway was developed to address. And whilst Hatchway does not use decorator-based routing, django-ninja-extra does! So I'm not seeing the connection beyond a vague notion of "features" as if they are some sort of commodity.

baseplate-admin commented 1 year ago

Hi thanks for responding to this issue


This issue feels a tad negative and misrepresentative.

I am sorry for that, my intention here was to give django-ninja a bit more love ( since theres a lot of open issues and open PR's )

Hatchway as a result of an oddity of the Mastodon API.

Fair enough. I wont argue here.

and at this rate most new projects will drop ninja.

This was written by my observation on my project as i had to monkeypatch some of django-ninja to allow better error catching Check : https://github.com/vitalik/django-ninja/issues/684 I also had to monkeypatch the optional auth ( which should be handled by django-ninja, along with permissions ).

Most importantly though, django-ninja-extra wouldn't address any of the concerns that you've raised.

So I'm not seeing the connection beyond a vague notion of "features" as if they are some sort of commodity.

I meant adding throttling, permissions and caching. Do you have a clear way to add these to a project when It's using django-ninja


A well-considered API design that's built to last is exactly what the community needs, given the current choices for building API-driven Django projects. Not to knock whoever is behind django-ninja-extra, but from even a quick glance it is obvious the package exists to add things that aren't proven enough to add to Django Ninja. Using it instead of Django Ninja for anything 'production-grade' is a questionable decision. It's not something I'd do myself.

It's not like that at all, vitalik pointed out he wanted to use django's built in functionality to recreate some of the functionality provided by the django-ninja-extra package ( to which i can say, its a better choice to support django instead of remaking the logics ourself ). Thats why codes from there aren’t merged into mainline repo.

But some of the work done by him is crawling its way into django-ninja ( for example the optional fields )


Overall i think django-ninja is a beta software at this point in time. I am hopeful it will become the de-facto rest implementation for django, but that period is markedly distinct from the present moment.

eadwinCode commented 1 year ago

I apologise if I came late to the party.

I think I will be changing my github primary email to a more personal email so that I wont be missing out on this kind of discussions.

The ideology behind the development of django-ninja-extra was from the point of building RESTful API in an object oriented fashion. Class based Approach is highly opinionated topic in this community. So django-ninja-extra exists because of this. It wasn't really designed to add what is missing in django-ninja but rather use django-ninja as a backbone for building cool features in the library.

@KyeRussell The initial thought process on django-ninja-extra permission system was for it to have similar interface as AuthBase so that users don't have to define permissions as parameter but rather use auth as a list of types or instances for both authentication and authorisation purposes. But I didnt go with this approach because I wanted to keep the operator feature of permission type e.g. (PermissionA && PermissionB).

A lot of features in django-ninja-extra was really inspired by DRF because I wanted people to easily migrate from DRF to django-ninja. And for anyone to easily convert DRF based libraries to be something that can work with django-ninja. The project is open for improvements as well.

I am just a big fan of django-ninja and just as @baseplate-admin said I also want django-ninja to be the de-facto rest implementation for django