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, pbl...@redhat.com Software Engineer _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev