Open Stack

On 23/09/13 21:59, Doug Hellmann wrote:
> On Mon, Sep 23, 2013 at 12:31 PM, Petr Blaho <pblaho at redhat.com> wrote:
> 
>> 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.

Hey Petr:


I believe you mentioned 'service models' yesterday :) on irc - well we
also used these in Deltacloud for the CIMI frontend - the API calls out
to the service models which handle talking to the database and
instantiating the separate object models. If understood correctly this
is also what Doug is proposing. I think it's a great idea.


My personal concern is getting the HK stories done without too much
disruption and this would be a major change. Though I also understand
that precisely for this reason we should do it sooner rather than later.
Do you think this is something you can work on in parallel to current
dev, rebasing as frequently as possible from master and aim to merge
immediately after HK? This is my vote anyway - thoughts? You could even
keep a gerrit review going and update that to encourage more people to look


thanks, marios



> 
> 
>>
>> 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.
>>
> 
> Yes, you definitely do not want to be using WSME models in the storage
> layer. In addition to the issues you've already discovered with the
> database schema not mapping directly to the API schema, it will force the
> storage code to depend on web front-end code.
> 
> In ceilometer, we solved this problem by creating a separate set of models
> for the storage layer:
> https://github.com/openstack/ceilometer/blob/master/ceilometer/storage/models.py
> 
> These are standalone objects, rather than sqlalchemy models, because we
> support multiple storage backends. At this point most of the objects are
> very simple data containers, but we used classes instead of dictionaries to
> ensure that all of the storage plugins behaved consistently (early on, we
> had a couple of drivers providing dictionaries with extra or missing keys).
> 
> The conversion between storage and API models is done in the API layer:
> https://github.com/openstack/ceilometer/blob/master/ceilometer/api/controllers/v2.py
> 
> The base WSME model has a "from_db_model()" method that lets you create an
> API object from a DB object. I could easily see creating a similar method
> to create a DB object from an API object, but it looks like we did that
> conversion within a Pecan controller method where we could validate the
> inputs more easily.
> 
> Doug
> 
> 
>>
>> 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
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
> 
> 
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>