Skip to content

Commit

Permalink
remove remaining == None
Browse files Browse the repository at this point in the history
sa does not allow is None in the whereclauses,
we create a simple null constant for usage inside the db apis

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
  • Loading branch information
Pierre Tardy committed Mar 8, 2014
1 parent 93daeb0 commit 83157ae
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 17 deletions.
3 changes: 3 additions & 0 deletions master/buildbot/db/__init__.py
Expand Up @@ -12,3 +12,6 @@
# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
#
# Copyright Buildbot Team Members

# a null constant to use in sqlalchemy whereclauses e.g. (tbl.c.results == null)
null = None
5 changes: 3 additions & 2 deletions master/buildbot/db/buildrequests.py
Expand Up @@ -17,6 +17,7 @@
import sqlalchemy as sa

from buildbot.db import base
from buildbot.db import null
from buildbot.util import datetime2epoch
from buildbot.util import epoch2datetime
from twisted.internet import reactor
Expand Down Expand Up @@ -82,11 +83,11 @@ def thd(conn):
if isinstance(claimed, bool):
if not claimed:
q = q.where(
(claims_tbl.c.claimed_at == None) &
(claims_tbl.c.claimed_at == null) &
(reqs_tbl.c.complete == 0))
else:
q = q.where(
(claims_tbl.c.claimed_at != None))
(claims_tbl.c.claimed_at != null))
else:
q = q.where(
(claims_tbl.c.masterid == claimed))
Expand Down
1 change: 1 addition & 0 deletions master/buildbot/db/builds.py
Expand Up @@ -16,6 +16,7 @@
import sqlalchemy as sa

from buildbot.db import base
from buildbot.db import null
from buildbot.util import epoch2datetime
from buildbot.util import json
from twisted.internet import reactor
Expand Down
7 changes: 4 additions & 3 deletions master/buildbot/db/buildsets.py
Expand Up @@ -20,6 +20,7 @@
import sqlalchemy as sa

from buildbot.db import base
from buildbot.db import null
from buildbot.util import datetime2epoch
from buildbot.util import epoch2datetime
from buildbot.util import json
Expand Down Expand Up @@ -127,7 +128,7 @@ def thd(conn):

q = tbl.update(whereclause=(
(tbl.c.id == bsid) &
((tbl.c.complete == None) | (tbl.c.complete != 1))))
((tbl.c.complete == null) | (tbl.c.complete != 1))))
res = conn.execute(q,
complete=1,
results=results,
Expand Down Expand Up @@ -157,7 +158,7 @@ def thd(conn):
q = q.where(bs_tbl.c.complete != 0)
else:
q = q.where((bs_tbl.c.complete == 0) |
(bs_tbl.c.complete == None))
(bs_tbl.c.complete == null))
res = conn.execute(q)
return [self._thd_row2dict(conn, row) for row in res.fetchall()]
return self.db.pool.do(thd)
Expand All @@ -180,7 +181,7 @@ def thd(conn):
q = q.where(bs_tbl.c.complete != 0)
else:
q = q.where((bs_tbl.c.complete == 0) |
(bs_tbl.c.complete == None))
(bs_tbl.c.complete == null))
if branch:
q = q.where(ss_tbl.c.branch == branch)
if repository:
Expand Down
5 changes: 3 additions & 2 deletions master/buildbot/db/changesources.py
Expand Up @@ -16,6 +16,7 @@
import sqlalchemy as sa

from buildbot.db import base
from buildbot.db import null
from twisted.internet import defer


Expand Down Expand Up @@ -86,9 +87,9 @@ def thd(conn):
if masterid is not None:
wc = (cs_mst_tbl.c.masterid == masterid)
elif active:
wc = (cs_mst_tbl.c.masterid != None)
wc = (cs_mst_tbl.c.masterid != null)
elif active is not None:
wc = (cs_mst_tbl.c.masterid == None)
wc = (cs_mst_tbl.c.masterid == null)

q = sa.select([cs_tbl.c.id, cs_tbl.c.name,
cs_mst_tbl.c.masterid],
Expand Down
Expand Up @@ -16,6 +16,7 @@
import migrate
import sqlalchemy as sa

from buildbot.db import null
from buildbot.util import sautils


Expand All @@ -32,7 +33,7 @@ def migrate_claims(migrate_engine, metadata, buildrequests, objects,
buildrequests.c.claimed_by_name.label("name"),
sa.literal_column("'BuildMaster'").label("class_name"),
],
whereclause=buildrequests.c.claimed_by_name != None,
whereclause=buildrequests.c.claimed_by_name != null,
distinct=True)

# this doesn't seem to work without str() -- verified in sqla 0.6.0 - 0.7.1
Expand All @@ -50,7 +51,7 @@ def migrate_claims(migrate_engine, metadata, buildrequests, objects,
objects.c.id.label('objectid'),
buildrequests.c.claimed_at,
], from_obj=[join],
whereclause=buildrequests.c.claimed_by_name != None)
whereclause=buildrequests.c.claimed_by_name != null)
migrate_engine.execute(
str(sautils.InsertFromSelect(buildrequest_claims, claims)))

Expand Down
5 changes: 3 additions & 2 deletions master/buildbot/db/schedulers.py
Expand Up @@ -17,6 +17,7 @@
import sqlalchemy.exc

from buildbot.db import base
from buildbot.db import null
from twisted.internet import defer


Expand Down Expand Up @@ -160,9 +161,9 @@ def thd(conn):
if masterid is not None:
wc = (sch_mst_tbl.c.masterid == masterid)
elif active:
wc = (sch_mst_tbl.c.masterid != None)
wc = (sch_mst_tbl.c.masterid != null)
elif active is not None:
wc = (sch_mst_tbl.c.masterid == None)
wc = (sch_mst_tbl.c.masterid == null)

q = sa.select([sch_tbl.c.id, sch_tbl.c.name,
sch_mst_tbl.c.masterid],
Expand Down
17 changes: 12 additions & 5 deletions master/docs/developer/database.rst
Expand Up @@ -8,11 +8,6 @@ backend. This section describes the database connector classes, which allow
other parts of Buildbot to access the database. It also describes how to
modify the database schema and the connector classes themselves.

.. note::

Buildbot is only half-migrated to a database backend. Build and builder
status information is still stored on disk in pickle files. This is
difficult to fix, although work is underway.

Database Overview
-----------------
Expand Down Expand Up @@ -1510,6 +1505,18 @@ Queries can be constructed using any of the SQLAlchemy core methods, using
tables from :class:`~buildbot.db.model.Model`, and executed with the connection
object, ``conn``.

.. note::

SQLAlchemy requires the use of a syntax that is forbidden by pep8.
If in where clauses you need to select rows where a value is null,
you need to write (`tbl.c.value == None`). This form is forbidden by pep8
which requires the use of `is None` instead of `== None`. As sqlalchemy is using operator
overloading to implement pythonic SQL statements, and `is` operator is not overloadable,
we need to keep the `==` operators. In order to solve this issue, buildbot
uses `buildbot.db.null` constant, which is `None`.
So instead of writting `tbl.c.value == None`, please write `tbl.c.value == null`)


.. py:class:: DBThreadPool
.. py:method:: do(callable, ...)
Expand Down
2 changes: 1 addition & 1 deletion master/docs/manual/cfg-schedulers.rst
Expand Up @@ -1226,7 +1226,7 @@ the new build. The new parameter is:
Example::

def get_compatible_builds(status, builder):
if builder == None: # this is the case for force_build_all
if builder is None: # this is the case for force_build_all
return ["cannot generate build list here"]
# find all successful builds in builder1 and builder2
builds = []
Expand Down

0 comments on commit 83157ae

Please sign in to comment.