Project

Profile

Help

Refactor #3555

closed

Remove "helper" viewset mixins

Added by dalley over 6 years ago. Updated almost 5 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
% Done:

100%

Estimated time:
Platform Release:
Groomed:
No
Sprint Candidate:
No
Tags:
Sprint:
Sprint 35
Quarter:

Description

In app/viewsets/base.py, we currently have the following "helper" mixins:

class NamedModelViewSet(mixins.CreateModelMixin,
                        mixins.RetrieveModelMixin,
                        mixins.DestroyModelMixin,
                        mixins.UpdateModelMixin,
                        mixins.ListModelMixin,
                        GenericNamedModelViewSet):
    """
    A viewset that provides default `create()`, `retrieve()`, `update()`, `partial_update()`,
    `destroy()` and `list()` actions.
    """
    pass

class CreateDestroyReadNamedModelViewSet(mixins.CreateModelMixin,
                                         mixins.RetrieveModelMixin,
                                         mixins.DestroyModelMixin,
                                         mixins.ListModelMixin,
                                         GenericNamedModelViewSet):
    """
    A customized NamedModelViewSet for models that don't support updates.

    A viewset that provides default `create()`, `retrieve()`, `destroy()` and `list()` actions.
    """
    pass

class CreateReadNamedModelViewSet(mixins.CreateModelMixin,
                                  mixins.RetrieveModelMixin,
                                  mixins.ListModelMixin,
                                  GenericNamedModelViewSet):
    """
    A customized NamedModelViewSet for models that don't support updates or deletes.

    A viewset that provides default `create()`, `retrieve()`, and `list()` actions.
    """
    pass

class CreateReadAsyncUpdateDestroyNamedModelViewset(mixins.CreateModelMixin,
                                                    mixins.RetrieveModelMixin,
                                                    mixins.ListModelMixin,
                                                    AsyncUpdateMixin,
                                                    AsyncRemoveMixin,
                                                    GenericNamedModelViewSet):
    """
    A viewset that performs asynchronous update and remove operations

    This viewset should be used with resources that require making a reservation for a repository
    during an update or delete.
    """
    pass

I propose deleting all of them, and renaming the "GenericNamedModelViewSet" to "NamedModelViewSet" for brevity.

Rationale:

  • The name "CreateReadAsyncUpdateDestroyNamedModelViewset" makes me want to burn my eyes out
  • It's not scalable
  • These helper mixins make it too easy to gloss over the individual actions being provided

I found a mistake [0] where the Worker model viewset inherited from CreateModelMixin, DeleteModelMixin, and UpdateModelMixin via the "NamedModelViewSet" helper. (This was prevented from being an actual bug by the fact that it manually specified the allowable HTTP method names).

Each and every viewset should enumerate all of its provided actions in a clear way. These helper mixins don't do that. Sure, it would make the viewset code a bit "uglier", but there's good ugly and there's bad ugly, and this is bad ugly IMO.

[0] https://github.com/pulp/pulp/pull/3413

Actions #1

Updated by dalley over 6 years ago

  • Sprint deleted (Sprint 35)
Actions #2

Updated by dalley over 6 years ago

  • Description updated (diff)
Actions #3

Updated by dalley over 6 years ago

  • Subject changed from Remove "helper" queryset mixins to Remove "helper" viewset mixins
  • Description updated (diff)
Actions #4

Updated by dalley over 6 years ago

  • Description updated (diff)
Actions #5

Updated by dalley over 6 years ago

  • Description updated (diff)
Actions #6

Updated by dalley over 6 years ago

  • Sprint set to Sprint 35
Actions #8

Updated by dalley over 6 years ago

  • Status changed from ASSIGNED to POST

Added by dalley over 6 years ago

Revision bfe9040f | View on GitHub

Remove 'helper' viewset mixins

closes #3555 https://pulp.plan.io/issues/3555

Added by dalley over 6 years ago

Revision bfe9040f | View on GitHub

Remove 'helper' viewset mixins

closes #3555 https://pulp.plan.io/issues/3555

Actions #9

Updated by dalley over 6 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100
Actions #10

Updated by daviddavis over 5 years ago

  • Sprint/Milestone set to 3.0.0
Actions #11

Updated by bmbouter over 5 years ago

  • Tags deleted (Pulp 3)
Actions #12

Updated by bmbouter almost 5 years ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE

Also available in: Atom PDF