← Back to team overview

nova team mailing list archive

Re: ORM Refactor

 

Well, that's not the only concern :)  The big concern, as I noted, is
that we're 3 weeks from code freeze and this patch touches almost all
the files in the code. ;)

-jay

On Fri, Sep 10, 2010 at 1:42 PM, Vishvananda Ishaya
<vishvananda@xxxxxxxxx> wrote:
> If the only concern is continuing to have a nosql solution, we should just add it.  It should only be a day or two of work to put it in.
>
> Vish
>
> On Sep 10, 2010, at 10:13 AM, Rick Clark wrote:
>
>> I think the abstraction is good, but I would rather wait until we have a
>> chance to add redis support.
>>
>>
>> On 09/10/2010 12:08 PM, Vishvananda Ishaya wrote:
>>> I don't understand the comment that this doesn't play nicely with
>>> non-relational data stores.  There is a very clear abstraction layer
>>> db/api.py that would allow a different backend to be plugged in
>>> regardless of whether sqlalchemy ever supports it.  I don't think it
>>> would be too difficult to add db/redis/api.py to this system.
>>>
>>> The one possible gotcha is that there isn't a clear definition of the
>>> properties that each object needs to have anywhere outside of
>>> sqlalchemy/models.py and the relations that are needed.  This
>>> ultimately should be fixed with either documentation or some kind of
>>> middle tier classes that define the needed properties.
>>>
>>> Delaying until after austin would be a bit troublesome for us since
>>> we have to move of of redis.  That means a pretty strongly diverged
>>> branch and any features that anso is working on will probably have to
>>> be delayed as well.
>>>
>>> Vish
>>>
>>> On Sep 10, 2010, at 9:56 AM, Jay Pipes wrote:
>>>
>>>> Hi Vish,
>>>>
>>>> Such a large patch has taken me quite some time to digest.  There
>>>> is a larger discussion on large patches without any specifications,
>>>> but I'll leave that for a later time! :)
>>>>
>>>> I am torn on this one, mostly because I spent a bunch of time
>>>> attempting to do the datastore refactoring myself (as did Justin
>>>> Santa Barbara), and thus I know the dragons that live in this layer
>>>> of the code :)
>>>>
>>>> One of the things that both Justin and I had tried was to keep an
>>>> abstraction layer that would allow both NoSQL as well as SQL data
>>>> stores to be used.  Unfortunately, it seems that this patch
>>>> removes the ability to use ReDIS, among other NoSQL stores.  I
>>>> think this is a mistake, and although I like much of the code in
>>>> this patch, I was hoping that SQLAlchemy could be hidden behind an
>>>> abstraction layer that would play nicely with the non-relational
>>>> data stores.
>>>>
>>>> As this patch stands, we take a 180 degree turn away from NoSQL
>>>> data stores and back into the relatively comfortable norms of the
>>>> SQL databases.  While there's nothing particularly wrong with SQL
>>>> databases (as you know, I'm a fan of many of them ;) ), I think
>>>> that keeping non-relational data store capabilities is pretty
>>>> critical.
>>>>
>>>> After an email discussion with SQLAlchemy's Michael Bayer about
>>>> SQLAlchemy's future with NoSQL data stores.  Although there is an
>>>> issue in the SQLAlchemy trac system about this (see here:
>>>> http://www.sqlalchemy.org/trac/ticket/1518) the likelihood of this
>>>> module seeing the light of day is unlikely in the next year or
>>>> two.
>>>>
>>>> So...what to do?  There are at least four options I can see:
>>>>
>>>> 1) Go forward with this patch and add NoSQL stores back at some
>>>> later time by ourselves 2) Go forward with this patch and wait
>>>> until SQLAlchemy properly supports key value stores 3) Delay this
>>>> patch until after the Austin release and have a larger discussion
>>>> about it here and at the next summit 4) Go back to the drawing
>>>> board and try again with a less ambitious set of patches that
>>>> incrementally changes the way the data stores work.
>>>>
>>>> I'm personally on the fence.  I'd prefer to at least delay the
>>>> patch until after Austin, but I understand there are now at least 4
>>>> branches that depend on this one, which makes things, well, a bit
>>>> difficult.
>>>>
>>>> -jay
>>>>
>>>> On Tue, Aug 31, 2010 at 8:46 PM, Vishvananda Ishaya
>>>> <vishvananda@xxxxxxxxx> wrote:
>>>>> I've proposed a merge of the orm refactor branch that a large
>>>>> part of the nasa/anso team has been working on.  I'm hoping
>>>>> everyone can pick it apart and we end up with a really clean
>>>>> system that everyone likes.  I've copied the description of the
>>>>> change and issues below.  If the mailing list debates get too
>>>>> complicated, we should just organize a time to discuss it in
>>>>> IRC.
>>>>>
>>>>> Proposing merge to get feedback on orm refactoring. I am very
>>>>> interested in feedback to all of these changes.
>>>>>
>>>>> This is a huge set of changes, that touches almost all of the
>>>>> files. I'm sure I have broken quite a bit, but better to take the
>>>>> plunge now than to postpone this until later. The idea is to
>>>>> allow for pluggable backends throughout the code.
>>>>>
>>>>> Brief Overview For compute/volume/network, there are multiple
>>>>> classes service - responsible for rpc this currently uses the
>>>>> existing cast and call in rpc.py and a little bit of magic to
>>>>> call public methods on the manager class. each service also
>>>>> reports its state into the database every 10 seconds manager -
>>>>> responsible for managing respective object classes all the
>>>>> business logic for the classes go here db (db_driver) -
>>>>> responsible for abstracting database access driver
>>>>> (domain_driver) - responsible for executing actual shell commands
>>>>> and implementation
>>>>>
>>>>> Compute hasn't been fully cleaned up, but to get an idea of how
>>>>> it works, take a look at volume and network
>>>>>
>>>>> Known issues/Things to be done:
>>>>>
>>>>> * nova-api accesses db objects directly It seems cleaner to have
>>>>> only the managers dealing with their respective objects. This
>>>>> would mean code for 'run_instances' would move into the manager
>>>>> class and it would do the initial setup and cast out to the
>>>>> remote service
>>>>>
>>>>> * db code uses flat methods to define its interface In my mind
>>>>> this is a little prettier as an abstract base class, but driver
>>>>> loading code can load a module or a class. It works, so I'm not
>>>>> sure it needs to be changed but feel free to debate it.
>>>>>
>>>>> * Service classes have no code in them Not sure if this is a
>>>>> problem for people, but the magic of calling the manager's
>>>>> methods is done in the base class. We could remove the magic from
>>>>> the base class and explicitly wrap methods that we want to make
>>>>> available via rpc if this seems nasty.
>>>>>
>>>>> * AuthManager Projects/Users/Roles are not integrated into this
>>>>> system. In order for everything to live happily in the backend,
>>>>> we need some type of adaptor for LDAP
>>>>>
>>>>> * Context is not passed properly across rabbit Context should
>>>>> probably be changed to a simple dictionary so that it can be
>>>>> passed properly through the queue
>>>>>
>>>>> * No authorization checks on access to objects We need to decide
>>>>> on which layer auth checks should happen.
>>>>>
>>>>> * Some of the methods in ComputeManager need to be moved into
>>>>> other layers/managers * Compute driver layer should be abstracted
>>>>> more cleanly * Flat networking is untested and may need to be
>>>>> reworked * Some of the api commands are not working yet * Nova
>>>>> Swift Authentication needs to be refactored(Todd is working on
>>>>> this)
>>>>>
>>>>> _______________________________________________ Mailing list:
>>>>> https://launchpad.net/~nova Post to     :
>>>>> nova@xxxxxxxxxxxxxxxxxxx Unsubscribe :
>>>>> https://launchpad.net/~nova More help   :
>>>>> https://help.launchpad.net/ListHelp
>>>>>
>>>>>
>>>
>>>
>>> _______________________________________________ Mailing list:
>>> https://launchpad.net/~nova Post to     : nova@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~nova More help   :
>>> https://help.launchpad.net/ListHelp
>>
>>
>
>



Follow ups

References