summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDan Callaghan <dcallagh@redhat.com>2018-08-08 17:02:00 +1000
committerDan Callaghan <dcallagh@redhat.com>2018-08-09 17:48:02 +1000
commit880b7656111c2f65fc4fda6306a9243f1b14bd7f (patch)
tree98d8332b5b5cf90af180eb21da4de0adc6164842
parent955843255e6e02f3dcf80a6c8f1d2baef89c6b0d (diff)
fix DECIMAL columns if their scale is wrong
The default MySQL precision and scale for DECIMAL (or Numeric in Python land) is 10,0 but for unexplained reasons the Red Hat production instance of Beaker has these columns as DECIMAL(10,2) instead. This patch makes the precision and scale explicit for all such columns, and adds extra checks to test_database_migration to catch inconsistencies. A migration is added to fix any old Beaker instances which have this problem. Bug: 1600281 Change-Id: Ia7b94c40fca81adb2c0075f1caf384f9e1c64aa3
-rw-r--r--IntegrationTests/src/bkr/inttest/server/test_database_migration.py6
-rw-r--r--IntegrationTests/src/bkr/inttest/server/test_model.py4
-rw-r--r--Server/bkr/server/alembic/migration_utils.py36
-rw-r--r--Server/bkr/server/alembic/versions/292b2e042673_fix_incorrect_decimal_scale.py37
-rw-r--r--Server/bkr/server/alembic/versions/41763e5d07cb_fix_job_product_id_type.py6
-rw-r--r--Server/bkr/server/model/inventory.py6
-rw-r--r--Server/bkr/server/model/scheduler.py7
7 files changed, 69 insertions, 33 deletions
diff --git a/IntegrationTests/src/bkr/inttest/server/test_database_migration.py b/IntegrationTests/src/bkr/inttest/server/test_database_migration.py
index 577f79f..65c431f 100644
--- a/IntegrationTests/src/bkr/inttest/server/test_database_migration.py
+++ b/IntegrationTests/src/bkr/inttest/server/test_database_migration.py
@@ -426,6 +426,12 @@ class MigrationTest(unittest.TestCase):
if hasattr(expected.type, 'length'):
self.assertEquals(actual.type.length, expected.type.length,
'%r has wrong length' % actual)
+ if hasattr(expected.type, 'precision'):
+ self.assertEquals(actual.type.precision, expected.type.precision,
+ '%r has wrong numeric precision' % actual)
+ if hasattr(expected.type, 'scale'):
+ self.assertEquals(actual.type.scale, expected.type.scale,
+ '%r has wrong numeric scale' % actual)
if hasattr(expected.type, 'enums'):
self.assertItemsEqual(actual.type.enums, expected.type.enums)
self.assertEquals(actual.nullable, expected.nullable,
diff --git a/IntegrationTests/src/bkr/inttest/server/test_model.py b/IntegrationTests/src/bkr/inttest/server/test_model.py
index 65a7020..12e3d4f 100644
--- a/IntegrationTests/src/bkr/inttest/server/test_model.py
+++ b/IntegrationTests/src/bkr/inttest/server/test_model.py
@@ -2666,9 +2666,9 @@ class RecipeTaskResultTest(DatabaseTestCase):
# https://bugzilla.redhat.com/show_bug.cgi?id=1600281
def test_caps_very_large_score(self):
- self.recipe_task.pass_(path=u'/distribution/kernelinstall/Sysinfo', score='123456789', summary=None)
+ self.recipe_task.pass_(path=u'/distribution/kernelinstall/Sysinfo', score='12345678900', summary=None)
session.refresh(self.recipe_task)
- self.assertEquals(self.recipe_task.results[0].score, 99999999)
+ self.assertEquals(self.recipe_task.results[0].score, 9999999999)
# https://bugzilla.redhat.com/show_bug.cgi?id=915319
def test_logs_appear_in_results_xml(self):
diff --git a/Server/bkr/server/alembic/migration_utils.py b/Server/bkr/server/alembic/migration_utils.py
index 5997b58..2f24132 100644
--- a/Server/bkr/server/alembic/migration_utils.py
+++ b/Server/bkr/server/alembic/migration_utils.py
@@ -71,6 +71,16 @@ def drop_fk(table,columns):
% (table, columns))
op.drop_constraint(constraint_name, table, type_='foreignkey')
+def find_column_type(table, column):
+ column_info = None
+ for info in sa.inspect(op.get_bind()).get_columns(table):
+ if info['name'] == column:
+ column_info = info
+ break
+ if not column_info:
+ raise AssertionError('%s.%s does not exist' % (table, column))
+ return column_info['type']
+
# When altering a MySQL ENUM column to add a new value, we can only add it at
# the end. Similarly values can only be removed from the end. The enum values
# must not be re-ordered, otherwise it will change the data stored in the
@@ -80,37 +90,23 @@ def drop_fk(table,columns):
# ALTER TABLE.
def add_enum_value(table, column, value, **kwargs):
- column_info = None
- for info in sa.inspect(op.get_bind()).get_columns(table):
- if info['name'] == column:
- column_info = info
- break
- if not column_info:
- raise AssertionError('%s.%s does not exist' % (table, column))
- column_info, = [info for info in sa.inspect(op.get_bind()).get_columns(table)
- if info['name'] == column]
- existing_enum_values = column_info['type'].enums
+ existing_type = find_column_type(table, column)
+ existing_enum_values = existing_type.enums
new_enum_values = existing_enum_values + (value,)
op.alter_column(table, column,
- existing_type=column_info['type'],
+ existing_type=existing_type,
type_=sa.Enum(*new_enum_values),
**kwargs)
def drop_enum_value(table, column, value, **kwargs):
- column_info = None
- for info in sa.inspect(op.get_bind()).get_columns(table):
- if info['name'] == column:
- column_info = info
- break
- if not column_info:
- raise AssertionError('%s.%s does not exist' % (table, column))
- existing_enum_values = column_info['type'].enums
+ existing_type = find_column_type(table, column)
+ existing_enum_values = existing_type.enums
if existing_enum_values[-1] != value:
raise AssertionError('Expecting to drop enum value %r from %s.%s, '
'but it is not the last value in the sequence! '
'Enum values are: %r' % (value, table, column, existing_enum_values))
new_enum_values = existing_enum_values[:-1]
op.alter_column(table, column,
- existing_type=column_info['type'],
+ existing_type=existing_type,
type_=sa.Enum(*new_enum_values),
**kwargs)
diff --git a/Server/bkr/server/alembic/versions/292b2e042673_fix_incorrect_decimal_scale.py b/Server/bkr/server/alembic/versions/292b2e042673_fix_incorrect_decimal_scale.py
new file mode 100644
index 0000000..d09d5f2
--- /dev/null
+++ b/Server/bkr/server/alembic/versions/292b2e042673_fix_incorrect_decimal_scale.py
@@ -0,0 +1,37 @@
+# This program 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; either version 2 of the License, or
+# (at your option) any later version.
+
+"""Fix columns with incorrect DECIMAL scale
+
+Very old Beaker instances may have some columns incorrectly as DECIMAL(10,2)
+instead of DECIMAL(10,0) as they are declared at the Python level.
+
+Revision ID: 292b2e042673
+Revises: 4d4e0a92ee10
+Create Date: 2018-08-08 16:34:11.942581
+"""
+
+# revision identifiers, used by Alembic.
+revision = '292b2e042673'
+down_revision = '4d4e0a92ee10'
+
+from alembic import op
+from sqlalchemy.types import Numeric
+from bkr.server.alembic.migration_utils import find_column_type
+
+def upgrade():
+ bad = [
+ ('labinfo', 'weight'),
+ ('labinfo', 'wattage'),
+ ('labinfo', 'cooling'),
+ ('recipe_task_result', 'score'),
+ ]
+ for table, column in bad:
+ column_type = find_column_type(table, column)
+ if column_type.scale != 0:
+ op.alter_column(table, column, type_=Numeric(10, 0))
+
+def downgrade():
+ pass # no downgrade as this was a schema mistake
diff --git a/Server/bkr/server/alembic/versions/41763e5d07cb_fix_job_product_id_type.py b/Server/bkr/server/alembic/versions/41763e5d07cb_fix_job_product_id_type.py
index 6ef8d47..1ba84d6 100644
--- a/Server/bkr/server/alembic/versions/41763e5d07cb_fix_job_product_id_type.py
+++ b/Server/bkr/server/alembic/versions/41763e5d07cb_fix_job_product_id_type.py
@@ -17,11 +17,11 @@ down_revision = '2c03c52950bf'
from alembic import op
import sqlalchemy as sa
+from bkr.server.alembic.migration_utils import find_column_type
def upgrade():
- column_info, = [info for info in sa.inspect(op.get_bind()).get_columns('job')
- if info['name'] == 'product_id']
- if not isinstance(column_info['type'], sa.Integer):
+ column_type = find_column_type('job', 'product_id')
+ if not isinstance(column_type, sa.Integer):
op.execute("""
UPDATE job
SET product_id = NULL
diff --git a/Server/bkr/server/model/inventory.py b/Server/bkr/server/model/inventory.py
index 5e57b7a..c7ff7b8 100644
--- a/Server/bkr/server/model/inventory.py
+++ b/Server/bkr/server/model/inventory.py
@@ -2381,9 +2381,9 @@ class LabInfo(DeclarativeMappedObject):
orig_cost = Column(Numeric(precision=16, scale=2, asdecimal=True))
curr_cost = Column(Numeric(precision=16, scale=2, asdecimal=True))
dimensions = Column(String(255))
- weight = Column(Numeric(asdecimal=False))
- wattage = Column(Numeric(asdecimal=False))
- cooling = Column(Numeric(asdecimal=False))
+ weight = Column(Numeric(10, 0, asdecimal=False))
+ wattage = Column(Numeric(10, 0, asdecimal=False))
+ cooling = Column(Numeric(10, 0, asdecimal=False))
fields = ['orig_cost', 'curr_cost', 'dimensions', 'weight', 'wattage', 'cooling']
diff --git a/Server/bkr/server/model/scheduler.py b/Server/bkr/server/model/scheduler.py
index 21fc53d..6531294 100644
--- a/Server/bkr/server/model/scheduler.py
+++ b/Server/bkr/server/model/scheduler.py
@@ -3867,7 +3867,7 @@ class RecipeTaskResult(TaskBase):
recipetask = relationship(RecipeTask, back_populates='results')
path = Column(Unicode(2048))
result = Column(TaskResult.db_type(), nullable=False, default=TaskResult.new)
- score = Column(Numeric(10))
+ score = Column(Numeric(10, 0))
log = Column(UnicodeText)
start_time = Column(DateTime, default=datetime.utcnow)
logs = relationship(LogRecipeTaskResult, back_populates='parent',
@@ -3875,10 +3875,7 @@ class RecipeTaskResult(TaskBase):
comments = relationship('RecipeTaskResultComment', back_populates='recipetaskresult')
#: Maximum allowable value for the score column.
- #: (This *should* be 10 digits since the column is Numeric(10), but due to
- #: a mistake in the Red Hat production database schema this is actually only
- #: 8 digits. This will be fixed in Beaker 26.0.)
- max_score = 99999999
+ max_score = 10 ** score.type.precision - 1
def __init__(self, recipetask=None, path=None, result=None,
score=None, log=None):