Skip to content

Commit

Permalink
Fix misleading error with empty table list #436
Browse files Browse the repository at this point in the history
  • Loading branch information
greenape committed Nov 7, 2022
1 parent a95b000 commit 053b0e7
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Fixed
- Fixed a potential deadlock when using a small connection pool and `store`-ing queries
- AutoFlow can now be run in a docker container with non-default user. [#5574](https://github.com/Flowminder/FlowKit/issues/5574)
- Passing an empty list of events tables when creating a query now raises `ValueError: Empty tables list.` instead of a `MissingDateError`. [#436](https://github.com/Flowminder/FlowKit/issues/436)

### Removed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,16 @@ def column_names(self) -> List[str]:
def _parse_tables(self, tables):
if tables is None:
return [f"events.{t}" for t in get_db().subscriber_tables]
elif isinstance(tables, str):
elif isinstance(tables, str) and len(tables) > 0:
return [tables]
elif isinstance(tables, str):
raise ValueError("Empty table name.")
elif not isinstance(tables, list) or not all(
[isinstance(tbl, str) for tbl in tables]
):
raise ValueError("Tables must be a string or list of strings.")
elif len(tables) == 0:
raise ValueError("Empty tables list.")
else:
return tables

Expand Down
23 changes: 23 additions & 0 deletions flowmachine/tests/test_events_table_union.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,29 @@ def test_get_only_sms(get_length):
assert get_length(etu) == 1246


@pytest.mark.parametrize(
"arg,error_type,error_message",
[
("", ValueError, "Empty table name."),
(0, ValueError, "Tables must be a string or list of strings."),
([0, "a"], ValueError, "Tables must be a string or list of strings."),
([], ValueError, "Empty tables list."),
],
)
def test_bad_table_arguments(arg, error_type, error_message):
"""
Test that an appropriate error is raised for bad tables arguments.
"""

with pytest.raises(expected_exception=error_type, match=error_message):
EventsTablesUnion(
"2016-01-01",
"2016-01-02",
columns=["msisdn"],
tables=arg,
)


def test_get_list_of_tables(get_length):
"""
Test that we can get only sms
Expand Down

0 comments on commit 053b0e7

Please sign in to comment.