pulp_mvc_architecture.md

Overview:

With the introduction of Mongoengine, we are creating a formal model layer in pulp. This allows us the opportunity to move code to better fit a traditional MVC design. Django and Mongoengine have differing ideas about MVC and many of the terms associated with it are overloaded, making any discussion potentially ambiguous. This proposal attempts to clarify which behaviors belong where and to define the flavor of MVC that Pulp will use going forward.

I have chosen Repository update as an example of how I intend separate the logic.

MVC

View:

Inherits from Django's View

pulp/server/webservices/views/{module}.py

Views receive and parse arguments from the API. They can interact with controllers to perform complex actions that involve multiple collections, or tasks. They use QuerySets to perform operations on a collection, and may use the model methods directly for simple actions. They serialize the results and return to the user. This logic will not move, but we will remove some code from the current views and move them to the models and controllers.

Views should not directly interact with tasks. Logic in the View should be minimal, leaving any heavy lifting to the controller and model.

Here is a somewhat simplified version of our the RepoResourceView update:

class RepoResourceView(View):

    def put(self, request, repo_id):

        # View code parses JSON body
        delta = request.body_as_json.get('delta', None)
        importer_config = request.body_as_json.get('importer_config', None)
        distributor_configs = request.body_as_json.get('distributor_configs', None)

        # QuerySet is used to retrieve (and validate existence) an object
        repo = model.Repository.objects.get_repo_or_missing_resource(repo_id)

        # The controller is given the arguments and does the work. This is simplified from
        # our actual code.
        raw_result = repo_controller.update_repo_and_plugins(repo, delta, importer_config, 
                                                         distributor_configs)

        # Views use custom serializers and convert into a JSON response.
        serialized_result = serializers.Repository(call_report['result']).data
        return generate_json_response_with_pulp_encoder(serialized_Result)

Model:

Inherits from Mongoengine's Document

All models will live in pulp/server/db/model.py, but for now, this is a package. So this code lives in pulp/server/db/model/__init__.py

The primary use of the model is exactly what we have been doing, defining fields, validations, etc. Additionally, 'model methods' can define operations on an individual object as long as those operations are synchronous and do not interact with other models.

The model methods should not use tasking or controllers. Usually, the use of multiple models suggests that the behavior belongs on the controller, but there may be exceptions.

class Repository(Document):

    fields = WhateverKindofField()
    meta = {set the queryset, indexes, and collection here}

    # Example of a (simplified) model method.
    def update_from_delta(delta):
        [setattr(self, key, value), for key, value in delta.items()]

CustomQuerySet:

Inherits from Mongoengine's QuerySet

pulp/server/db/querysets.py

Operations on the whole collection can be defined here. The name can be confusing because a Mongoengine QuerySet is different from a Django QuerySet. We will be using these as a "manager that acts on the collection as a whole".

class RepositoryQuerySet(CriteriaQuerySet):

    def get_repo_or_missing_resource(self, repo_id):
        try:
            return self.get(repo_id=repo_id)
        except DoesNotExists:
            raise MissingResource(repository=repo_id)

Controller:

Operations that use other models, managers, and controllers are defined here.

pulp/server/controllers/{module}.py

Additionally, tasks are defined and queued here. The reason for this is that the tasks perform operations on the models and controllers (and managers for now). Thia code will primarily come from the managers but some of it will move from pulp.server.tasks.<collection>.

Note: These are totally different from the web.py controllers.

def update_repo_and_plugins(repo, repo_delta, importer_config, distributor_configs):

    # Use the model for simple interactions.
    if repo_delta:
        if isinstance(repo_delta, dict):
            repo.update_from_delta(repo_delta)
            repo.save()
        else:
            raise pulp_exceptions.InvalidValue(['delta'])

    # Use other controllers (or managers) when necessary.
    if importer_config is not None:
        importer_manager = manager_factory.repo_importer_manager()
        importer_manager.update_importer_config(repo.repo_id, importer_config)

    # Task queuing should happen here also.
    additional_tasks = []
    if distributor_configs is not None:
        for dist_id, dist_config in distributor_configs.items():
            task_tags = [
                tags.resource_tag(tags.RESOURCE_REPOSITORY_TYPE, repo.repo_id),
                tags.resource_tag(tags.RESOURCE_REPOSITORY_DISTRIBUTOR_TYPE,
                                  dist_id),
                tags.action_tag(tags.ACTION_UPDATE_DISTRIBUTOR)
            ]
            async_result = dist_controller.update.apply_async_with_reservation(
                tags.RESOURCE_REPOSITORY_TYPE, repo.repo_id,
                [repo.repo_id, dist_id, dist_config, None], tags=task_tags)
            additional_tasks.append(async_result)
    return TaskResult(repo, None, additional_tasks)

Additionally, tasks related to a model should be defined here. This does not happen with update, so I will use sync as the example here:

# Queue tasks with appropriate tags.
def queue_sync_with_auto_publish(repo_id, overrides=None):
    kwargs = {'repo_id': repo_id, 'sync_config_override': overrides}
    tags = [resource_tag(RESOURCE_REPOSITORY_TYPE, repo_id), action_tag('sync')]
    result = sync.apply_async_with_reservation(RESOURCE_REPOSITORY_TYPE, repo_id, tags=tags,
                                               kwargs=kwargs)
    return result

# Define tasks and name them based on where they were for backwards compatibility. These names will
# be changed all at once, this is tracked here: https://pulp.plan.io/issues/1052.
@celery.task(base=Task, name='pulp.server.managers.repo.sync.sync')
def sync(repo_id, sync_config_override=None):
    repo_obj = model.Repository.objects.get_repo_or_missing_resource(repo_id)
    transfer_repo = repo_obj.to_transfer_repo()
    # Rest of the code left out for simplicity. The important part is that the logic to sync a repo
    # and inform the other controllers/managers is here. 

Nice work. This appears to be solid to me!

One note though: I think we don't need to say that only views use controllers. There may be other users of them in the future. Perhaps you meant that Managers and Models should not use Controllers?

This is a really good doc. I hope it becomes part of our actual documentation at some point. Maybe an task could be put in to do that.

One thing to note is that moving a task from where it lives today to a method on a Controller needs to be done with care. Especially, upgrades that move or delete tasks need to ensure that there are no tasks lingering in the system.

Looks good. Nice write up.

To bmbouter's point, it is especially important that the python path for schedulable tasks does not change., because they have long-lived references formed from their python path.

minor point, but Repository.update_from_delta and RepositoryQuerySet.get_repo_or_missing_resource are missing self as the first parameter. ;)

Actually, I think there are several other syntax errors and similar, so I'll just trust that when these docs become part of an official doc, you'll use real-world code examples. :)