← Back to team overview

launchpad-dev team mailing list archive

Re: Looking for persistence layer dedicated reviewer(s).

 

On 22 December 2010 16:21, Robert Collins <robert.collins@xxxxxxxxxxxxx> wrote:
> @everyone: I couldn't stop thinking about the basic issues, so
> lp:~lifeless/launchpad/persistence has a brain-dump of the
> contract-level design and some mild worked examples, feedback
> appreciated.

(The guts are in
<http://bazaar.launchpad.net/~lifeless/launchpad/persistence/annotate/head:/lib/lp/persistence/doc/index.txt>).

> * Explicit actions whenever possibly expensive operations will occur.
> * Multiple object / object graph operations.

+1

In your description of layering, is there supposed to be another layer
that describes how model objects map to a store?  Is this direct from
model object to a particular store, or do you map to something
abstracted across stores, or both?

In the examples it would be nice to see a worked example of defining a
new model class.  For example if we wanted to track (uh I'm lost for
examples because Launchpad tracks so many things) but let's say a
person's plane tickets, we would add:

  a PlaneTicket model object?
  a mapping from that to psql?  is this inline with the model or separate?
  the SQL DDL to add it to the db? or is this inferred?
  a query class for tickets?  or is there a sufficient generic default?

>    bug = Bug(launchbag.user, launchbag.context)

I guess user is there just as 'filed by this user'?

>     transaction.add(user, bug)

I realize it's near-pseudocode but I think it's better not to pass
lists of arbitrary things as *args.

    query = tranasction.make_query()
    query.projects.filter = ByName('bzr')
    return query.projects.bugs.execute()

It seems odd that traversing the 'projects' attribute on the query
makes it be a query about projects, if that's what the example is
supposed to convey.  Also unfortunate to need to declare a new class
ByName?  This style seems to make it hard to select, say, tasks that
touch both bzr and launchpad, even though you could say that easily
enough in sql.

What's returned?  An iterable of projects?

If we want good control over when the db is touched it seems useful to
be able to piggyback together queries that return similar data used by
different bits of code.  (This may not always actually be faster but
perhaps it's worth allowing for doing it.)  So that seems to mean its
useful to let the result a query pull back many objects, and then to
get just a particular aspect of them, and therefore it should give
back something richer than just an iterable.

It seems like to describe a query you need to describe what parts of
the object graph you wish to be able to traverse in the result.  You
need to know ahead of time, because we want to avoid people going back
to the db to ask for just a bit more and just a bit more.  This
delineation is in two dimensions, at least: which relation attributes
you want to traverse, and which values of them are wanted, if not all.

It may be worth explicitly mentioning (if true) that db access only
happens during .execute(), .commit(), and perhaps a few other
operations.  You never get lazy query results back.

    def getConjoinedMaster(bugtask_collection_query, *args, **kwargs):
        """Expand bug_collection_query to permit getConjoinedMaster calls.

        See `lp.bugs.model.bugtask.BugTask.getConjoinedMaster` for the
        arguments accepted by this call.
        """
        # We need to access the bugtasks and their series
        bugtask_collection_query.distroseries
        # and their product series
        bugtask_collection_query.productseries
        # and the distribution and its currentseries
        bugtask_collection_query.distribution.currentseries
        # and we access products too
        bugtask_collection_query.product.development_focus

what happens to the starargs? Are they somehow magically read out?

It looks like you're saying merely touching an attribute causes it to
be brought in.  That's kind of cool, but perhaps too magical.  I
wonder too if it will cause trouble you want to specify

  bug.assignee == Person
  bug.reporter == Person

Is that an 'and' or an 'or'?  You might reasonably want either.


It seems one key thing about queries is that

    query = transaction.make_query()
    query.bugs.tasks.project.ByName('bzr')
    query.bugs.execute()

I wonder should that be query.execute().bugs?  And say
query.bugs.tasks.project.where('name', Equals, 'bzr').

hth

-- 
Martin



Follow ups

References