Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix misleading error with empty table list #4367

Merged
merged 1 commit into from
Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,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