Open Stack

Hi,

during my work on getting tests to pass for
https://review.openstack.org/#/c/46947/ I discovered that we are
misusing pecan models for HTTP representation of Resources.

In controllers pecan/wsme calls actions with pecan model prepopulated
from HTTP request's params.

For example, when creating new Rack, post
method in Racks controller is called with rack object
( https://github.com/stackforge/tuskar/blob/master/tuskar/api/controllers/v1/rack.py#L26-L31 ).
This object is instance of Rack from
https://github.com/stackforge/tuskar/blob/master/tuskar/api/controllers/v1/types/rack.py
.
Next this object is used in pecan.request.dbapi.create_rack(rack) call
(method defined here
https://github.com/stackforge/tuskar/blob/master/tuskar/db/sqlalchemy/api.py#L385-L431
)

This method just assumes that new_rack object (passed from controller)
has some attributes with getters defined (name, subnet, location, ...).

This is fine if we have 1-1 relationship between how resource is stored
in db and how it is represented via HTTP. In fact this assumes that both
versions have the same set of atributes.

Marios wrote a patch (mentioned above) which needs some internal
attributes only, ie. present in table but not exposed via HTTP.

In that moment I found that we use pecan models (
https://github.com/stackforge/tuskar/tree/master/tuskar/api/controllers/v1/types )
to transfer HTTP params _directly_ to our db layer
( https://github.com/stackforge/tuskar/blob/master/tuskar/db/sqlalchemy/api.py ).
By _directly_ I mean that we assume 1-1 mapping between attributes in
pecan models and db models.

This is not to be true anymore. We can solve it by using conditionals like
this
https://review.openstack.org/#/c/46947/3/tuskar/db/sqlalchemy/api.py
(lines 175 to 181) but I think this is not good solution b/c of repetion
of code and generaly we are mixing up HTTP repr. with db repr.

I propose to write a simple layer responsible for "translating" pecan
models into db representation. This way we can keep all diffs in what
attributes are exposed via HTTP and which not in one place - easy to
see, easy to change, easy to review. To scatter it around dbapi code
is not a good way IMHO.


Another issue which comes with this coupling of pecan/db models is in
tests.

In db tests we use utility helpers for creating resources in memory, ie
create_test_resource_class method (
https://github.com/stackforge/tuskar/blob/master/tuskar/tests/db/test_db_racks.py#L28
). This kind of methods comes from "from tuskar.tests.db import utils"
and they use pecan models (
https://github.com/stackforge/tuskar/blob/master/tuskar/tests/db/utils.py#L17-L23 ).
We are now on the same page as mentioned above. These db tests uses
Relation and Link pecan models which means that easy solution like
importing db models instead of pecan models is not doable at the moment
b/c db models do not contains direct counterparts for Relation and Link.


I am pretty woried about this pecan-db models coupling b/c if (or when)
we will need more different stuctures on HTTP side than on db side (no
1-1 relationship) we will have to change our code and tests more.

I hope that we will find a solution for this sooner than tuskar code
will grow more complex.


I would like to see something like "service objects" in controller part
but simple set of "translations" can be ok if we do not break 1-1 parity too
much.

Tests will require more attention b/c of that Relation and Link pecan
objects.


Thank you for your patience with reading this and looking for a
feedback. Maybe I missed something or I see this bigger than it really
is or I am totally out :-)

-- 
Petr Blaho, pblaho at redhat.com
Software Engineer