← Back to team overview

nova team mailing list archive

Re: ORM Refactor

 

I think someone covered this but our main rationale behind moving from redis is that it doesn't have sufficient security for our needs.  If a machine were to be compromised, redis is essentially just wide open.

While I can certainly understand not wanting to put a major patch like this in near release, I would also like to avoid if at all possible having to keep our code that we must use for Nebula 1.0 launch from being so diverged from trunk, as that will cause a lot more problems in the long run.

Fortunately we have pretty good test coverage and we've been using the orm_deux branch fairly extensively in the past few weeks.  It is my personal opinion that we should move forward with this merge, as I think it will save us a lot of headaches overall.


Devin




On Sep 10, 2010, at 10:47 AM, Jay Pipes wrote:

> 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
>>> 
>>> 
>> 
>> 
> 
> _______________________________________________
> 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