Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3197 DB schema: change_properties to text #2561

Merged
merged 1 commit into from
Jan 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions master/buildbot/db/changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ def thd(conn):
for i in inserts:
self.checkLength(tbl.c.property_name,
i['property_name'])
self.checkLength(tbl.c.property_value,
i['property_value'])

conn.execute(tbl.insert(), inserts)
if uid:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# This file is part of Buildbot. Buildbot is free software: you can
# redistribute it and/or modify it under the terms of the GNU General Public
# License as published by the Free Software Foundation, version 2.
#
# This program is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
# details.
#
# You should have received a copy of the GNU General Public License along with
# this program; if not, write to the Free Software Foundation, Inc., 51
# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
#
# Copyright Buildbot Team Members

from __future__ import absolute_import
from __future__ import print_function
import sqlalchemy as sa
from migrate import changeset


def upgrade(migrate_engine):
metadata = sa.MetaData()
metadata.bind = migrate_engine

if migrate_engine.dialect.name == "postgresql":
# changeset.alter_column has no effect on postgres, so we do this with raw sql
migrate_engine.execute("alter table change_properties alter column property_value type text")

else:
# Commit messages can get too big for the normal 1024 String limit.
changeset.alter_column(
sa.Column('property_value', sa.Text, nullable=False),
table='change_properties',
metadata=metadata,
engine=migrate_engine)
2 changes: 1 addition & 1 deletion master/buildbot/db/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ class Model(base.DBConnectorComponent):
nullable=False),
sa.Column('property_name', sa.String(256), nullable=False),
# JSON-encoded tuple of (value, source)
sa.Column('property_value', sa.String(1024), nullable=False),
sa.Column('property_value', sa.Text, nullable=False),
)

# users associated with this change; this allows multiple users for
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change properties 'value' changed from String(1024) to Text. Requires upgrade master. (:bug:`3197`)
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# This file is part of Buildbot. Buildbot is free software: you can
# redistribute it and/or modify it under the terms of the GNU General Public
# License as published by the Free Software Foundation, version 2.
#
# This program is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
# details.
#
# You should have received a copy of the GNU General Public License along with
# this program; if not, write to the Free Software Foundation, Inc., 51
# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
#
# Copyright Buildbot Team Members

from __future__ import absolute_import
from __future__ import print_function
from random import choice
from string import ascii_lowercase
import sqlalchemy as sa
from twisted.trial import unittest
from buildbot.test.util import migration
from buildbot.util import sautils


class Migration(migration.MigrateTestMixin, unittest.TestCase):

def setUp(self):
return self.setUpMigrateTest()

def tearDown(self):
return self.tearDownMigrateTest()

def create_table_thd(self, conn):
metadata = sa.MetaData()
metadata.bind = conn

change_properties = sautils.Table(
'change_properties', metadata,
sa.Column('changeid', sa.Integer, nullable=False),
sa.Column('property_name', sa.String(256), nullable=False),
sa.Column('property_value', sa.String(1024), nullable=False),
)

change_properties.create()

def test_update(self):
def setup_thd(conn):
self.create_table_thd(conn)

def verify_thd(conn):
metadata = sa.MetaData()
metadata.bind = conn
random_length = 65535
random_string = ''.join(choice(ascii_lowercase) for byte in range(random_length))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well "x"*4096 would have been enough..


# Verify column type is text
change_properties = sautils.Table('change_properties', metadata, autoload=True)
self.assertIsInstance(change_properties.c.property_value.type, sa.Text)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for sqlite, we dont really care as TEXT and VARCHAR are stored the same anyway. We rather need to check that the property can store > 1024 string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense. i will add this test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think you added that test


# Test write and read random string
conn.execute(change_properties.insert(), [dict(
changeid=1,
property_name="test_change_properties_property_value_length",
property_value=random_string,
)])
q = conn.execute(sa.select(
[change_properties.c.property_value]).where(change_properties.c.changeid == 1))
[self.assertEqual(q_string[0].encode("ascii"), random_string) for q_string in q]

return self.do_test_migration(47, 48, setup_thd, verify_thd)