[Buildbot-devel] schedulerdb progress report: week 1

[Kristian Nielsen]

Brian Warner <warner at lothar.com> writes:

> Just an update on what I've gotten done so far:
>
> * http://github.com/warner/buildbot/tree/schedulerdb contains the new
>   code, branched off of 'master'

Thanks for allowing us a chance to see what you're doing!

> The new database schema is in buildbot/db.py . I wrote a wrapper around

Some comments/suggestions:

> TABLES = [
>     # the schema here is defined as version 1
>     """CREATE TABLE version
>     (
>     version INTEGER -- contains one row, currently set to 1
>     );""",
>     """INSERT INTO version VALUES (1);""",
> 
>     # last_access is used for logging, to record the last time that each
>     # client (or rather class of clients) touched the DB. The idea is that if
>     # something gets weird, you can check this and discover that you have an
>     # older tool (which uses a different schema) mucking things up.
>     """CREATE TABLE last_access
>     (
>     who VARCHAR(256), -- like 'buildbot-0.8.0'
>     writing INTEGER, -- 1 if you are writing, 0 if you are reading
>     last_access TIMESTAMP     -- seconds since epoch
>     );""",

Sounds like you intend here PRIMARY KEY (who, writing) ?

(I don't know how primary keys work with sqlite, but I'll add comments for
primary keys to use for databases like MySQL, Postgress, etc.).

>     """CREATE TABLE changes_nextid (next_changeid INTEGER);""",
>     """INSERT INTO changes_nextid VALUES (0);""",

Hm, what is the purpose of this table? Is it something specific to sqlite to
get AUTOINCREMENT working, not needed in other databases?

>     """CREATE TABLE changes
>     (
>     changeid INTEGER PRIMARY KEY AUTOINCREMENT,
>     number INTEGER UNIQUE,

Why 'number' ? How is it different from changeid?

>From your code, looks like this is from the old code, asigned by Buildbot. You
really should not have two different unique ids. If Buildbot already maintains
'number', then why not use it in place of 'changeid' ?

>     author VARCHAR(1024),
>     comments VARCHAR(1024), -- too short?

Should be TEXT/BLOB. Possibly you could put it in a different
(changeid,comments) table, though that's probably premature optimisation.

>     is_dir INTEGER, -- old, for CVS
>     branch VARCHAR(1024),
>     revision VARCHAR(256), -- too short for darcs?
>     revlink VARCHAR(256),

What is 'revlink'?

>     when_timestamp INTEGER, -- or TIMESTAMP?. Copied from incoming Change.
>     category VARCHAR(256)
>     );""",
> 
>     """CREATE TABLE change_links
>     (
>     changeid INTEGER,
>     link VARCHAR[1024]
>     );""",

Is this a link to the associated page on eg. github or Launchpad?

Is the link unique, or can there be many. In other words what is the primary
key?

If it is unique, make changeid the primary key. Else add a field 'idx
INTEGER', and use PRIMARY KEY (changeid, idx).

(No need for an AUTOINCREMENT, as the code can easily number idx as
0,1,2,3... within each changeid).

> 
>     """CREATE TABLE change_files
>     (
>     changeid INTEGER,
>     filename VARCHAR[1024]
>     );""",

Ok, files are not unique, so add 'idx INTEGER' and use PRIMARY KEY (changeid,
idx).

>     """CREATE TABLE change_properties
>     (
>     changeid INTEGER,
>     property_name VARCHAR[256],
>     property_value VARCHAR[1024] -- too short?
>     );""",
>     ]

PRIMARY KEY (changeid, property_name) here seems to make sense.

----

Hope this helps,

 - Kristian.