minor improvements in error handling and testing

src/infi/clickhouse_orm/database.py

@@ -116,9 +116,10 @@ class Database(object):
         '''
         Creates a table for the given model class, if it does not exist already.
         '''
-        # TODO check that model has an engine
         if model_class.system:
             raise DatabaseException("You can't create system table")
+        if getattr(model_class, 'engine') is None:
+            raise DatabaseException("%s class must define an engine" % model_class.__name__)
         self._send(model_class.create_table_sql(self))
 
     def drop_table(self, model_class):

src/infi/clickhouse_orm/engines.py

@@ -40,6 +40,8 @@ class MergeTree(Engine):
         assert date_col is None or isinstance(date_col, six.string_types), 'date_col must be string if present'
         assert partition_key is None or type(partition_key) in (list, tuple),\
             'partition_key must be tuple or list if present'
+        assert (replica_table_path is None) == (replica_name == None), \
+            'both replica_table_path and replica_name must be specified'
 
         # These values conflict with each other (old and new syntax of table engines.
         # So let's control only one of them is given.
@@ -52,7 +54,6 @@ class MergeTree(Engine):
         self.index_granularity = index_granularity
         self.replica_table_path = replica_table_path
         self.replica_name = replica_name
-        # TODO verify that both replica fields are either present or missing
 
     # I changed field name for new reality and syntax
     @property

src/infi/clickhouse_orm/fields.py

@@ -39,7 +39,7 @@ class Field(object):
         data can't be converted. Returns the converted value. Subclasses should override this.
         The timezone_in_use parameter should be consulted when parsing datetime fields.
         '''
-        return value
+        return value   # pragma: no cover
 
     def validate(self, value):
         '''

tests/base_test_with_data.py

@@ -21,8 +21,8 @@ class TestCaseWithData(unittest.TestCase):
         self.database.drop_table(Person)
         self.database.drop_database()
 
-    def _insert_and_check(self, data, count):
-        self.database.insert(data)
+    def _insert_and_check(self, data, count, batch_size=1000):
+        self.database.insert(data, batch_size=batch_size)
         self.assertEquals(count, self.database.count(Person))
         for instance in data:
             self.assertEquals(self.database, instance.get_database())

tests/test_database.py

@@ -2,7 +2,7 @@
 from __future__ import unicode_literals
 import unittest
 
-from infi.clickhouse_orm.database import ServerError
+from infi.clickhouse_orm.database import ServerError, DatabaseException
 from .base_test_with_data import *
 
 
@@ -20,6 +20,12 @@ class DatabaseTestCase(TestCaseWithData):
     def test_insert__empty(self):
         self._insert_and_check([], 0)
 
+    def test_insert__small_batches(self):
+        self._insert_and_check(self._sample_data(), len(data), batch_size=10)
+
+    def test_insert__medium_batches(self):
+        self._insert_and_check(self._sample_data(), len(data), batch_size=100)
+
     def test_count(self):
         self.database.insert(self._sample_data())
         self.assertEquals(self.database.count(Person), 100)
@@ -150,3 +156,11 @@ class DatabaseTestCase(TestCaseWithData):
     def test_preexisting_db(self):
         db = Database(self.database.db_name, autocreate=False)
         db.count(Person)
+
+    def test_missing_engine(self):
+        class EnginelessModel(Model):
+            float_field = Float32Field()
+        with self.assertRaises(DatabaseException) as cm:
+            self.database.create_table(EnginelessModel)
+        self.assertEqual(cm.exception.message, 'EnginelessModel class must define an engine')
+

tests/test_engines.py

@@ -51,6 +51,12 @@ class EnginesTestCase(_EnginesHelperTestCase):
             expected = "ReplicatedMergeTree('/clickhouse/tables/{layer}-{shard}/hits', '{replica}', date, (date, event_id, event_group), 8192)"
         self.assertEquals(engine.create_table_sql(self.database), expected)
 
+    def test_replicated_merge_tree_incomplete(self):
+        with self.assertRaises(AssertionError):
+            MergeTree('date', ('date', 'event_id', 'event_group'), replica_table_path='/clickhouse/tables/{layer}-{shard}/hits')
+        with self.assertRaises(AssertionError):
+            MergeTree('date', ('date', 'event_id', 'event_group'), replica_name='{replica}')
+
     def test_collapsing_merge_tree(self):
         class TestModel(SampleModel):
             engine = CollapsingMergeTree('date', ('date', 'event_id', 'event_group'), 'event_version')