Skip to content

[WIP] Channels v2 Support #42

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

Closed
wants to merge 17 commits into from

Conversation

hishnash
Copy link

@hishnash hishnash commented Feb 14, 2018

This is a very much work in progress

I'm opening it so we can discuss our implementation. #41

It might be good if there is a channelsv2 branch that we can do pull-requests to so the multiple people can contribute to this big upgrade without making master dirty and then just replace master once we are ready for a full release.

Testing

Transactions

the @action decorator that must be added to all action methods now includes an atomic option.

class Users(GenericAsyncAPIConsumer):
    queryset = get_user_model().objects.all()
    serializer_class = UserSerializer

    @action(atomic=True)
    def test_sync_action(self, pk=None):
        user = self.get_object(pk=pk)

        s = self.get_serializer(
            action_kwargs={'pk': pk},
            instance=user
        )
        return s.data, 200   

this value will default to the Django settings ATOMIC_REQUESTS option if not set.

Observers

this is not quite like bindings. It lets you subscribe to a given Django signal so that this signal will be sent to all instances of the consumer.

class TestConsumer(AsyncJsonWebsocketConsumer):

    async def accept(self):
        await super().accept()
        await self.handle_user_logged_in.subscribe()

    @observer(user_logged_in)
    async def handle_user_logged_in(self, *args, **kwargs):
        await self.send_json({'message': kwargs,})

Model Observers

like signal observers @decorate a method on your consumer.

class TestConsumer(AsyncJsonWebsocketConsumer):

    async def accept(self):
        await self.user_change.subscribe()
        await super().accept()

    @model_observer(get_user_model())
    async def user_change(self, message):
        await self.send_json(message)

TODO

  • Bundle as part of a transation (only send the msg when the transation is complete)

Bindings

For now, it might be best if the first implementation of this just handles actions fired through the channels-API.

I need to do some testing on were the pre-save post-save etc triggers are fired in Django when using twisted core-routines as channels v2 does for the entire app, are they triggered on all core-routines of this prosses or just in the routines that creates them?

but if we want to go with the full observation of Django signals... then:

Channels v2 does not include the Bindings module so my suggestion would be:

Pull the code from v1 into channels-API and adapt it to work with channels v2. But this could result in a license conflict (I'm not an expert)

django-channels is BSD 3-Clause "New" or "Revised" License.

Channels-API is either BSD (as listed in settup.py) or MIT as listed in LICENSE

alternatively, we can try to bring over the binding functions without a direct code copy just by being inspired by the same concepts.... (no idea if this is would hold up in court but those features be nice)

Django View As Consumer

I believe a nice feature to have is the ability to set up a consumer that will route actions to HTTP verbs and trigger a dedicated Django/DRF view.

DjangoViewAsConsumer and view_as_consumer convenience method are examples of how this might be used.

applciation =  view_as_consumer(MyDRFViewSet.asView({'get': 'user_info', 'post': 'new_user'})

how should we handle errors in DRF, if we do nothing they will be returned in the data key rather than the errors key as they would normally return from other endpoints?

GenericAsyncAPIConsumer

like DFR GenericAPIView this is the base class for all the model action view mixins.

yes a shameless copy of DRF.

  • Add credits to on this to DRF authors.

Just like DRF there are a set of mixins that can be used here:
CreateModelMixin
ListModelMixin
RetrieveModelMixin
UpdateModelMixin
PatchModelMixin
DeleteModelMixin

Pagination

at the moment I have removed this we need to add it back:

there are a few different ways we could do this
a) like it was before

b) Add the support to stream data out in chunks.
eg you send 1 request and you will get back many responses each with N items, with a configurable delay between them. (this might be nice for Pagination of binding updates)

Async Vs Sync

channels v2 supports the async await from Python 3.5.

so the new @action() decorator will detect if the method is async if not it will wrap it inside the needed database_sync_to_async wrapper so that the consumer can call it.

Other changes?

please list other changes we should make as we do this since this will require a large re-write of a lot of the code to work within the new channels v2 apis/ASGI application framework.

@hishnash hishnash force-pushed the channels-v2-support branch from e1263b3 to ace569f Compare February 19, 2018 08:26
@hishnash hishnash force-pushed the channels-v2-support branch from e781e08 to 82a1338 Compare February 20, 2018 07:01
@hishnash hishnash force-pushed the channels-v2-support branch from 1bfd490 to 91fecd4 Compare February 20, 2018 14:32
@hishnash hishnash force-pushed the channels-v2-support branch from 0a97f69 to 2454d5a Compare February 21, 2018 07:38
@hishnash hishnash force-pushed the channels-v2-support branch 2 times, most recently from 0be68b7 to f1d1ac0 Compare March 1, 2018 09:31
@hishnash hishnash force-pushed the channels-v2-support branch from f1d1ac0 to aa90024 Compare March 1, 2018 09:39
@linuxlewis
Copy link
Owner

This PR feels premature. I'd like to wait and see how channels 2.0 stabilizes before jumping into a entirely new pattern. Also you are making unrequired infrastructure to the project that I disagree with. If migrating to channels 2.0 requires rewriting the whole package then maybe it's time for a new package.

@linuxlewis linuxlewis closed this Mar 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants