wemake-services / wemake-django-rest

Create Django REST APIs the right way, no magic intended
MIT License
11 stars 1 forks source link

Corelation with dry python project. #3

Open proofit404 opened 5 years ago

proofit404 commented 5 years ago

Hi, we discussed some of this in private during Moscow Python Conf 2018.

I've got the ideas of this framework this way:

I have the same design goals in mind when I write integration layers for dry-python

Here is an example of Django REST Framework usage

from dependencies import Injector
from dependencies.contrib.rest_framework import model_view_set

@model_view_set
class BlogPosts(Injector):

    serializer_class = BlogPostSerializer
    permission_classes = ...

    create = create_blog_post 
    update = update_blog_post
    destroy = remove_blog_post

In this example, the actual ModelViewSet class will be created during registration in the router.

The only thing user can do - attach callables to the API scenarios.

The user is not allowed to access view set base class.

What do you think?

Regards, Artem.

sobolevn commented 5 years ago

Hi @proofit404 !

Yes, you got the main idea, but the are several other things that is very important:

  1. No magic! I don't like too much magic in my code. And drf is full of magic, so I try to reduce it
  2. Typing support, we will use @dataclasses for containing data, so we will be able to improve mypy checks for the end user
  3. Separation of business validation and typing
  4. swagger-first, everything should be translated into swagger spec as is, with little or no modification from user, this is quite easy for us, since we use declarative API
  5. Clean routing
  6. Graceful error handling for different steps in our application
  7. No global settings

Considering integration parts. I can see two points where we can both benefit:

  1. controller callable which can be a dry-python primitive enforcing best practices
  2. we can use dependencies to setup proper Declaration instances, since it may have a lot of dependent composed classes

I will try to work on this library on the next week, after I will release a major update for wemake-python-styleguide.

Any feedback is welcome!

proofit404 commented 5 years ago

Thanks for so verbose response!

I take so time to think about it. Here what I came to:

Magic.

I prefer as little magic as possible instead of no magic at all.

We use things attrs and django.db.models. They are built on tons of magic. But we use them anyway because we benefit from this libraries a lot.

I think about dry-python the same way. If there will be a huge improvement in development experience using stories and dependencies, many people will tolerate the magic behind it.

But we should keep the amount of the magic to the bare minimum.

Swagger.

I like the idea, but it out of scope of the dry-python project itself. There is no place for it in stories or dependencies.

Routing.

We provide dependencies.contrib packages for Django, Rest Framework and Celery. The hard requirement for each package to be tranparrent for framawork routing. For example, this is how it looks like to add DI container to the Django routing:

path("shop/", login_required(views.CategoryShopView.as_view()), name="category-shop"),

It look and feal quite the same as regular view.

Error handling.

Each dry python project tries it best to find error made by user. Each failure we found we provide wide error message with explanation of failure reasons.

DependencyError( "'Injector' can not resolve attribute 'test' while building 'bar'")
DependencyError("'foo' is a circular dependency in the 'Bar' constructor")
DependencyError("'foo' argument can not have class as its default value")

We are planning to provide suggestions for user to apply to the code base.

Settings.

There is no global parts in dependencies or stories.

Results.

After our conversition I made few changes to stories

Here is an example of it:

@dataclass
class Context:
    category_id: int
    price_id: int
    user: UserModel
    category: CategoryModel

class BuySubscription:

    @story
    @argument("category_id")
    @argument("price_id")
    @argument("user")
    def buy(self):

        self.find_category()
        self.find_price()
        self.find_profile()
        self.check_balance()

    def find_category(self, ctx: Context):

        category = load_category(ctx.category_id)
        return Success(category=category)

def load_category(category_id: int) -> CategoryModel:
    return CategoryModel.objects.get(pk=category_id)
proofit404 commented 5 years ago

Also, I came to an interesting idea of separate parts of DSL by name in the documentation.

I want users to keep in mind that self in the story, self in the method and ctx are all different things.

I will try this layout:

class BuySubscription:

    @story
    @argument("category_id")
    @argument("price_id")
    @argument("user")
    def buy(I):

        I.find_category()
        I.find_price()
        I.find_profile()
        I.check_balance()

    def find_category(self, ctx: Context):

        category = load_category(ctx.category_id)
        return Success(category=category)

What do you think?

sobolevn commented 5 years ago

I like that now we have explicit context! That's awesome!

Considering the whole design. I have some questions about your motivation behind these ideas.

  1. You said that the main reason behind @story being a decorator is that you can use it with any class, so you can have these operations in db.Model, or any other class with a metaclass. This seems like a good thing! But why you have dropped the idea of an operation as a separate class? Like this example:
from dry import Story

class BuySubscription(Story): ...

# And then it can later be used in a model:

class Subscription(db.Model):
     def buy(self):
          return BuySubscription(self.id).buy()

Or something similar.

  1. Why do you want your pipeline to be defined as function calls? This way you have multiple problems along side: you have to mention that these method have different selfs, that sometimes it will be called with parameters, sometimes not. And in the end we have two magic things: I.find_category() does not match the function definition, and will brake things like pycharm, and maybe mypy. And then someone is calling this function with patched behavior. That might be confusing as well. What do I suggest instead?
from dry import Pipeline, story

@dataclass
class Context:
    category_id: int
    price_id: int
    user: User

class BuySubscription:
     @story
     def buy(self, category_id: int, price_id: int, user: User) -> Pipeline[Context]:  # generic
            return Pipeline(context=Context(category_id, price_id, user), steps=[
                 self.find_category,
                 self.find_price,
                 ....
            ])

    def find_category(self, ctx: Context) -> Monad[Context]:  # return type is not clear to me yet
        category = load_category(ctx.category_id)
        return Success(category=category)

And then we can use it like so:

service = BuySubscription()
service.buy(1, 2, user).run()  # could be any method of `Pipeline`

This seems like pure python code to me. But, I would like to hear your thoughts on these. Cheers!

proofit404 commented 5 years ago

Thanks for the reply!

I didn't drop the idea of the separate class! I just didn't have time to think it enough. But I will.

Both suggestions seem reasonable to me. I'll give them a try.

My main idea behind DSL design was:

A story should be as readable as possible, line by line, without syntax noise.

Function call was just the first satisfactory idea which came to my mind.

I agree that it has a problem with a broken signature.

The call is optional by the way

class BuySubscription:

    @story
    @argument("category_id")
    @argument("price_id")
    @argument("user")
    def buy(I):

        I.find_category
        I.find_price
        I.find_profile
        I.check_balance

    def find_category(self, ctx: Context):

        category = load_category(ctx.category_id)
        return Success(category=category)

Regards, Artem.

proofit404 commented 5 years ago

With DSL expressed in class attributes it can be something like:

class BuySubscription(Story):

    category_id: argument
    price_id: argument
    user: argument

    find_category: step
    find_price: step
    find_profile: step
    check_balance: step
sobolevn commented 5 years ago

@proofit404 I love the idea that the code should be as readable as possible. Just to be clear, are you showing this code to your business people? This seems like a good motivation for this usecase.

But, if not, I would love to see more moving parts of the library as a developer. This will be much more readable for me. Since I will be able to debug it, go to definition and so on.

Some final words about readability. I would love to see something that generates gherkin-like feature files from these features. Like you have already done for pytest and toolbar. But, this is my personal thing. Since we write a lot of gherkin files for the business owners we work with: https://wemake.services/meta/rsdp/requirements-analysis/#user-stories

Considering your class design, this looks great. But, will this syntax allow us to see what step comes after what step? I assumed that class attributes do not contain any specific ordering.

proofit404 commented 5 years ago

Thanks for so useful feedback!

I agree that many people will be skeptical about DSL expressed in magic method calls from nowhere.

I guess we can go the same way as SQLAlchemy does.

stories.core will provide Story(context=Context(), steps=[foo, bar, baz]) lowlevel api.

stories.dsl.* will provide fancy declarative API. Maybe few of them (current version and classes version).

I really like the gherkin generation. I create a ticket for it https://github.com/dry-python/stories/issues/74

I think the class design is possible with Python 3 metaclass prepare method.