(core) Show proper message on empty Excel import, rather than a code error

Summary:
- Previously showed "UnboundLocalError". Now will show:
    Import failed: Failed to parse Excel file.
    Error: No tables found (1 empty tables skipped)
- Also fix logging for import code

Test Plan: Added a test case

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3396
This commit is contained in:
Dmitry S 2022-04-26 01:50:57 -04:00
parent dcafa96b5b
commit e59dcc142d
4 changed files with 14 additions and 3 deletions

View File

@ -111,6 +111,7 @@ const cssModalBody = styled('div', `
padding: 16px 0; padding: 16px 0;
overflow-y: auto; overflow-y: auto;
max-width: 470px; max-width: 470px;
white-space: pre-line;
`); `);
const cssModalHeader = styled('div', ` const cssModalHeader = styled('div', `

View File

@ -141,7 +141,7 @@ export class DocPluginManager {
'.csv' : 'CSV', '.csv' : 'CSV',
}; };
const fileType = extToType[path.extname(fileName)] || path.extname(fileName); const fileType = extToType[path.extname(fileName)] || path.extname(fileName);
throw new Error(`Failed to parse ${fileType} file. Error: ${messages.join("; ")}`); throw new Error(`Failed to parse ${fileType} file.\nError: ${messages.join("; ")}`);
} }
throw new Error(`File format is not supported.`); throw new Error(`File format is not supported.`);
} }

View File

@ -40,7 +40,7 @@ def parse_file(file_path, orig_name, parse_options=None, table_name_hint=None, n
return parse_open_file(f, orig_name, table_name_hint=table_name_hint) return parse_open_file(f, orig_name, table_name_hint=table_name_hint)
except Exception as e: except Exception as e:
# Log the full error, but simplify the thrown error to omit the unhelpful extra args. # Log the full error, but simplify the thrown error to omit the unhelpful extra args.
log.info("import_xls parse_file failed: %s", e) log.warn("import_xls parse_file failed: %s", e)
if six.PY2 and e.args and isinstance(e.args[0], six.string_types): if six.PY2 and e.args and isinstance(e.args[0], six.string_types):
raise Exception(e.args[0]) raise Exception(e.args[0])
raise raise
@ -58,6 +58,7 @@ def parse_open_file(file_obj, orig_name, table_name_hint=None):
if table_set.encoding == 'ascii': if table_set.encoding == 'ascii':
table_set.encoding = 'utf8' table_set.encoding = 'utf8'
skipped_tables = 0
export_list = [] export_list = []
# A table set is a collection of tables: # A table set is a collection of tables:
for row_set in table_set.tables: for row_set in table_set.tables:
@ -101,6 +102,7 @@ def parse_open_file(file_obj, orig_name, table_name_hint=None):
if not table_data: if not table_data:
# Don't add tables with no columns. # Don't add tables with no columns.
skipped_tables += 1
continue continue
log.info("Output table %r with %d columns", table_name, len(column_metadata)) log.info("Output table %r with %d columns", table_name, len(column_metadata))
@ -112,8 +114,12 @@ def parse_open_file(file_obj, orig_name, table_name_hint=None):
"table_data": table_data "table_data": table_data
}) })
parse_options = {} if not export_list:
if skipped_tables:
raise Exception("No tables found ({} empty tables skipped)".format(skipped_tables))
raise Exception("No tables found")
parse_options = {}
return parse_options, export_list return parse_options, export_list

View File

@ -7,6 +7,7 @@ import sys
sys.path.append('thirdparty') sys.path.append('thirdparty')
# pylint: disable=wrong-import-position # pylint: disable=wrong-import-position
import logging
import marshal import marshal
import functools import functools
@ -25,6 +26,9 @@ from imports.register import register_import_parsers
import logger import logger
log = logger.Logger(__name__, logger.INFO) log = logger.Logger(__name__, logger.INFO)
# Configure logging module to behave similarly to logger. (It may be OK to get rid of logger.)
logging.basicConfig(format="[%(levelname)s] [%(name)s] %(message)s")
def table_data_from_db(table_name, table_data_repr): def table_data_from_db(table_name, table_data_repr):
if table_data_repr is None: if table_data_repr is None:
return actions.TableData(table_name, [], {}) return actions.TableData(table_name, [], {})