Code review comment for lp:~rackspace-titan/glance/wsgi-refactor

Revision history for this message
Jay Pipes (jaypipes) wrote :

> Perhaps Serializer is not the right name for this set of functionality. But I
> hope you will agree that "Controller" is a bit too broad of a term to fit into
> a system with a good division of responsibility.

Not really... it's called MVC for a reason... The Controller deals with logic and business rules, and that's what *most* of the stuff that was placed into the ImageSerializer was doing. And I think that is incorrect. Logic should stay in the Controller. The Controller should create and populate a Response object and return it. Middleware (both paste-constructed and manually created as in the case of serializers) should modify the Response object. That's it. No logic.

> I am certainly open to
> renaming both. But maybe we ought not rename Controller, so that we remain
> consistent with other such units across Openstack.

I wasn't suggesting we rename Controller :) Like I said, it's an integral part of the MVC pattern.

> However, our naming mistake
> doesn't require us to make a similar mistake in the division of responsibility
> just to conform.

I don't understand this, sorry. I think I've explained why a piece of middleware should not be constructing the Response object. That's the responsibility of the center-most object in the chain: the Controller.

> Although the naming is extremely important, if we set it aside I believe that
> leaving it to the "postprocessor" of the controller's output (currently the
> serializer) to create the WebOb response is a powerful approach--it lets the
> Controller focus on the problem domain rather than on wsgi details.

Postprocessing what? Oh, that's right... postprocessing the *Response* object :) Postprocessing implies that you are processing a previously-constructed object, no? ;)

> I hope some of that makes sense! :-)

Not really, but I still dig you man :)

-jay

« Back to merge proposal