Skip to content

Commit

Permalink
Log exceptions and fix erroneous handling (#46)
Browse files Browse the repository at this point in the history
  • Loading branch information
DavidHuber-NOAA authored Nov 20, 2024
1 parent a7b49e9 commit 3ea5acd
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 22 deletions.
2 changes: 1 addition & 1 deletion src/wxflow/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def _true_or_not(string: str):

try:
return to_datetime(string) # Try as a datetime
except Exception as exc:
except Exception:
if string in BOOLS: # Likely a boolean, convert to True/False
return _true_or_not(string)
elif '.' in string: # Likely a number and that too a float
Expand Down
4 changes: 2 additions & 2 deletions src/wxflow/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def _copy_files(filelist, required=True):
logger.info(f'Copied {src} to {dest}')
except Exception as ee:
logger.exception(f"Error copying {src} to {dest}")
raise ee(f"Error copying {src} to {dest}")
raise ee
else:
if required:
logger.exception(f"Source file '{src}' does not exist and is required, ABORT!")
Expand All @@ -109,4 +109,4 @@ def _make_dirs(dirlist):
logger.info(f'Created {dd}')
except Exception as ee:
logger.exception(f"Error creating directory {dd}")
raise ee(f"Error creating directory {dd}")
raise ee
7 changes: 4 additions & 3 deletions src/wxflow/fsutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,10 @@ def cp(source: str, target: str) -> None:
try:
shutil.copy2(source, target)
except OSError:
raise OSError(f"unable to copy {source} to {target}")
except Exception as exc:
raise Exception(exc)
raise OSError(f"Unable to copy {source} to {target}")
except Exception as ee:
logger.exception(f"An unknown error occurred while copying {source} to {target}")
raise ee


# Group ID number for a given group name
Expand Down
3 changes: 2 additions & 1 deletion src/wxflow/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,8 @@ def _get_key_name(key):
sub_schema, is_main_schema=False, description=_get_key_description(key)
)
if isinstance(key, Optional) and hasattr(key, "default"):
expanded_schema[key_name]["default"] = _to_json_type(_invoke_with_optional_kwargs(key.default, **kwargs) if callable(key.default) else key.default) # nopep8
expanded_schema[key_name]["default"] = (_to_json_type(_invoke_with_optional_kwargs(key.default, **kwargs)
if callable(key.default) else key.default))
elif isinstance(key_name, Or):
# JSON schema does not support having a key named one name or another, so we just add both options
# This is less strict because we cannot enforce that one or the other is required
Expand Down
13 changes: 7 additions & 6 deletions src/wxflow/sqlitedb.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import sqlite3
from logging import getLogger
from typing import Any, List, Optional, Tuple

__all__ = ["SQLiteDB"]

logger = getLogger(__name__.split('.')[-1])


class SQLiteDBError(Exception):
"""
Expand Down Expand Up @@ -35,10 +38,7 @@ def connect(self) -> None:
"""

try:
self.connection = sqlite3.connect(self.db_name)
except sqlite3.OperationalError as exc:
raise sqlite3.OperationalError(exc)
self.connection = sqlite3.connect(self.db_name)

def disconnect(self) -> None:
"""
Expand Down Expand Up @@ -110,13 +110,14 @@ def remove_column(self, table_name: str, column_name: str) -> None:
try:
query = f"ALTER TABLE {table_name} DROP COLUMN {column_name}"
self.execute_query(query)
except sqlite3.OperationalError as exc:
except sqlite3.OperationalError as ee:
query = f"PRAGMA table_info({table_name})"
cursor = self.execute_query(query)
columns = [column[1] for column in cursor.fetchall()]
if column_name not in columns:
raise ValueError(f"Column '{column_name}' does not exist in table '{table_name}'")
raise sqlite3.OperationalError(exc)
logger.exception(f"Failed to remove '{column_name}' from '{table_name}' due to an unknown error.")
raise ee

def update_data(
self,
Expand Down
40 changes: 32 additions & 8 deletions tests/test_file_utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import os

import pytest

from wxflow import FileHandler


Expand Down Expand Up @@ -27,6 +29,12 @@ def test_mkdir(tmp_path):
assert os.path.exists(dd)


def test_bad_mkdir():
# Attempt to create a directory in an unwritable parent directory
with pytest.raises(OSError):
FileHandler({'mkdir': ["/dev/null/foo"]}).sync()


def test_copy(tmp_path):
"""
Test for copying files:
Expand All @@ -35,6 +43,7 @@ def test_copy(tmp_path):
tmp_path - pytest fixture
"""

# Test 1 (nominal operation) - Creating a directory and copying files to it
input_dir_path = tmp_path / 'my_input_dir'

# Create the input directory
Expand All @@ -56,7 +65,7 @@ def test_copy(tmp_path):
for src, dest in zip(src_files, dest_files):
copy_list.append([src, dest])

# Create config object for FileHandler
# Create config dictionary for FileHandler
config = {'copy': copy_list}

# Copy input files to output files
Expand All @@ -66,17 +75,32 @@ def test_copy(tmp_path):
for ff in dest_files:
assert os.path.isfile(ff)

# Create a config object for copying optional non-existent files (c.txt does not exist)
# Test 2 - Attempt to copy files to a non-writable directory
# Create a list of bad targets (/dev/null is unwritable)
bad_dest_files = ["/dev/null/a.txt", "/dev/null/bb.txt"]

bad_copy_list = []
for src, dest in zip(src_files, bad_dest_files):
bad_copy_list.append([src, dest])

# Create a config dictionary for FileHandler
bad_config = {'copy': bad_copy_list}

# Attempt to copy
with pytest.raises(OSError):
FileHandler(bad_config).sync()

# Test 3 - Attempt to copy missing, optional files to a writable directory
# Create a config dictionary (c.txt does not exist)
copy_list.append([input_dir_path / 'c.txt', output_dir_path / 'c.txt'])
config = {'copy_opt': copy_list}

# Copy input files to output files
# Copy input files to output files (should not raise an error)
FileHandler(config).sync()

# Create a config object for copying required non-existent files (c.txt does not exist)
# Test 4 - Attempt to copy missing, required files to a writable directory
# Create a config dictionary (c.txt does not exist)
config = {'copy_req': copy_list}
try:
c_file = input_dir_path / 'c.txt'
with pytest.raises(FileNotFoundError, match=f"Source file '{c_file}' does not exist"):
FileHandler(config).sync()
except FileNotFoundError as e:
c_file = input_dir_path / 'c.txt'
assert f"Source file '{c_file}' does not exist" in str(e)
2 changes: 1 addition & 1 deletion tests/test_sqlitedb.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def test_remove_column(db):
column_name = "address"
db.remove_column("test_table", column_name)

# Verify that the column exists in the test table
# Verify that the column no longer exists in the test table
assert not column_exists(db, "test_table", column_name)


Expand Down

0 comments on commit 3ea5acd

Please sign in to comment.