Pass the request into managers

[dstufft]

Currently, the manager interface more or less assumes that it can operate completely independently of the request object and doesn't pass it in. You can see this in code like:

class Create(ResourceBase):
    """
    A base class to extend that allows for
    adding the create ability to your resource.
    """
    __abstract__ = True

    @apimethod(methods=['POST'], no_pks=True)
    @manager_translate(validate=True, fields_attr='create_fields')
    def create(cls, request):
        """
        Creates a new resource using the cls.manager.create
        method.  Returns the resource that was just created
        as an instance of the class that created it.
        :param RequestContainer request: The request in the standardized
            ripozo style.
        :return: An instance of the class
            that was called.
        :rtype: Update
        """
        _logger.debug('Creating a resource using manager %s', cls.manager)
        props = cls.manager.create(request.body_args)
        meta = dict(links=dict(created=props))
        return cls(properties=props, meta=meta, status_code=201)

This forces managers that need to access something off of the request (such as, the database transaction associated with this current request) to use something like thread local storage to smuggle the currently active transaction into the manager. Global state and indirect passing is a bit of an anti pattern and I generally prefer to pass things like that explicitly into the methods.

The resources themselves generally support this just fine since each of those methods accepts a request object, but the same isn't the true for managers (and by extension, any of the code that calls a manager, like the CRUD mixins).

If instead a function was called with the request object to get these, that would make it possible to do this cleanly. For example:

class ResourceBase:

    @classmethod
    def get_manager(cls, request):
        return self.manager

class Create(ResourceBase):

    __abstract__ = True

    @apimethod(methods=['POST'], no_pks=True)
    @manager_translate(validate=True, fields_attr='create_fields')
    def create(cls, request):
        """
        Creates a new resource using the cls.manager.create
        method.  Returns the resource that was just created
        as an instance of the class that created it.
        :param RequestContainer request: The request in the standardized
            ripozo style.
        :return: An instance of the class
            that was called.
        :rtype: Update
        """
        _logger.debug('Creating a resource using manager %s', cls.manager)
        props = cls.get_manager(request).create(request.body_args)
        meta = dict(links=dict(created=props))
        return cls(properties=props, meta=meta, status_code=201)

This would mean that by default, folks who assigned their manager to Resource.manager will have everything still work, but folks like me who want to have explicit request bound sessions can then do something like:

from ripozo_sqlalchemy import AlchemyManager, SessionHandler


class PersonManager(AlchemyManager):
    model = Person
    fields = ('id', 'first_name', 'last_name')


class RequestBoundManager:
    @classmethod
    def get_manager(cls, request):
        return PersonManager(SessionHandler(request.db)

And then things should just work, without needing to use thread locals to smuggle that information in.

[timmartin19]

This seems appropriate to me. I intentionally didn't pass the request object but I could see why you may want to instantiate your manager with request specific arguments. If you can write up a pull request to do this in a backwards compatible manner I would be more than happy to merge it. The backwards compatibility doesn't have to be beautiful code. I plan on removing the shims in V2.0.0