Merge pull request #222 from datafold/aug30

Better error messages. Move some parsing to before the connects.

Author: erezsh

data_diff/__main__.py

@@ -155,6 +155,8 @@ def _main(
             logging.debug(f"Applied run configuration: {__conf__}")
     elif verbose:
         logging.basicConfig(level=logging.INFO, format=LOG_FORMAT, datefmt=DATE_FORMAT)
+    else:
+        logging.basicConfig(level=logging.WARNING, format=LOG_FORMAT, datefmt=DATE_FORMAT)
 
     if limit and stats:
         logging.error("Cannot specify a limit when using the -s/--stats switch")
@@ -181,14 +183,6 @@ def _main(
             logging.error("Error: threads must be >= 1")
             return
 
-    db1 = connect(database1, threads1 or threads)
-    db2 = connect(database2, threads2 or threads)
-    dbs = db1, db2
-
-    if interactive:
-        for db in dbs:
-            db.enable_interactive()
-
     start = time.monotonic()
 
     try:
@@ -199,7 +193,7 @@ def _main(
             where=where,
         )
     except ParseError as e:
-        logging.error("Error while parsing age expression: %s" % e)
+        logging.error(f"Error while parsing age expression: {e}")
         return
 
     differ = TableDiffer(
@@ -210,6 +204,25 @@ def _main(
         debug=debug,
     )
 
+    if database1 is None or database2 is None:
+        logging.error(
+            f"Error: Databases not specified. Got {database1} and {database2}. Use --help for more information."
+        )
+        return
+
+    try:
+        db1 = connect(database1, threads1 or threads)
+        db2 = connect(database2, threads2 or threads)
+    except Exception as e:
+        logging.error(e)
+        return
+
+    dbs = db1, db2
+
+    if interactive:
+        for db in dbs:
+            db.enable_interactive()
+
     table_names = table1, table2
     table_paths = [db.parse_table_name(t) for db, t in safezip(dbs, table_names)]
 

data_diff/databases/connect.py

@@ -202,4 +202,4 @@ def connect(db_conf: Union[str, dict], thread_count: Optional[int] = 1) -> Datab
         return connect_to_uri(db_conf, thread_count)
     elif isinstance(db_conf, dict):
         return connect_with_dict(db_conf, thread_count)
-    raise TypeError(db_conf)
+    raise TypeError(f"db configuration must be a URI string or a dictionary. Instead got '{db_conf}'.")

tests/common.py

@@ -63,12 +63,17 @@ def get_git_revision_short_hash() -> str:
     db.Trino: TEST_TRINO_CONN_STRING,
 }
 
-for k, v in CONN_STRINGS.items():
-    if v is None:
-        logging.warn(f"Connection to {k} not configured")
-    else:
-        logging.info(f"Testing database: {k}")
 
+def _print_used_dbs():
+    used = {k.__name__ for k, v in CONN_STRINGS.items() if v is not None}
+    unused = {k.__name__ for k, v in CONN_STRINGS.items() if v is None}
+
+    logging.info(f"Testing databases: {', '.join(used)}")
+    if unused:
+        logging.info(f"Connection not configured; skipping tests for: {', '.join(unused)}")
+
+
+_print_used_dbs()
 CONN_STRINGS = {k: v for k, v in CONN_STRINGS.items() if v is not None}
 
 

tests/test_database_types.py

@@ -26,11 +26,18 @@
     _drop_table_if_exists,
 )
 
+CONNS = None
 
-CONNS = {k: db.connect.connect(v, N_THREADS) for k, v in CONN_STRINGS.items()}
 
-CONNS[db.MySQL].query("SET @@session.time_zone='+00:00'", None)
-oracle.SESSION_TIME_ZONE = postgresql.SESSION_TIME_ZONE = "UTC"
+def init_conns():
+    global CONNS
+    if CONNS is not None:
+        return
+
+    CONNS = {k: db.connect.connect(v, N_THREADS) for k, v in CONN_STRINGS.items()}
+    CONNS[db.MySQL].query("SET @@session.time_zone='+00:00'", None)
+    oracle.SESSION_TIME_ZONE = postgresql.SESSION_TIME_ZONE = "UTC"
+
 
 DATABASE_TYPES = {
     db.PostgreSQL: {
@@ -374,7 +381,7 @@ def __iter__(self):
         ) in source_type_categories.items():  # int, datetime, ..
             for source_type in source_types:
                 for target_type in target_type_categories[type_category]:
-                    if CONNS.get(source_db, False) and CONNS.get(target_db, False):
+                    if CONN_STRINGS.get(source_db, False) and CONN_STRINGS.get(target_db, False):
                         type_pairs.append(
                             (
                                 source_db,
@@ -518,6 +525,9 @@ def _create_table_with_indexes(conn, table, type):
 class TestDiffCrossDatabaseTables(unittest.TestCase):
     maxDiff = 10000
 
+    def setUp(self) -> None:
+        init_conns()
+
     def tearDown(self) -> None:
         if not BENCHMARK:
             _drop_table_if_exists(self.src_conn, self.src_table)

tests/test_diff_tables.py

@@ -20,8 +20,17 @@
     N_THREADS,
 )
 
+DATABASE_INSTANCES = None
 DATABASE_URIS = {k.__name__: v for k, v in CONN_STRINGS.items()}
-DATABASE_INSTANCES = {k.__name__: connect(v, N_THREADS) for k, v in CONN_STRINGS.items()}
+
+
+def init_instances():
+    global DATABASE_INSTANCES
+    if DATABASE_INSTANCES is not None:
+        return
+
+    DATABASE_INSTANCES = {k.__name__: connect(v, N_THREADS) for k, v in CONN_STRINGS.items()}
+
 
 TEST_DATABASES = {x.__name__ for x in (db.MySQL, db.PostgreSQL, db.Oracle, db.Redshift, db.Snowflake, db.BigQuery)}
 
@@ -56,6 +65,7 @@ def _get_text_type(conn):
         return "STRING"
     return "varchar(100)"
 
+
 def _get_float_type(conn):
     if isinstance(conn, db.BigQuery):
         return "FLOAT64"
@@ -79,6 +89,7 @@ class TestPerDatabase(unittest.TestCase):
 
     def setUp(self):
         assert self.db_name
+        init_instances()
 
         self.connection = DATABASE_INSTANCES[self.db_name]
         if self.with_preql:
@@ -215,10 +226,12 @@ def setUp(self):
         float_type = _get_float_type(self.connection)
 
         self.connection.query(
-            f"create table {self.table_src}(id int, userid int, movieid int, rating {float_type}, timestamp timestamp)", None
+            f"create table {self.table_src}(id int, userid int, movieid int, rating {float_type}, timestamp timestamp)",
+            None,
         )
         self.connection.query(
-            f"create table {self.table_dst}(id int, userid int, movieid int, rating {float_type}, timestamp timestamp)", None
+            f"create table {self.table_dst}(id int, userid int, movieid int, rating {float_type}, timestamp timestamp)",
+            None,
         )
         # self.preql(
         #     f"""