Skip to content

Commit

Permalink
Small CLI fixes (#234)
Browse files Browse the repository at this point in the history
A couple args were broken in the 2.1 release:
--umls-key (env var would work, but not CLI arg)
--builder
  • Loading branch information
mikix authored May 10, 2024
1 parent 528f3c0 commit 6866553
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 31 deletions.
10 changes: 5 additions & 5 deletions cumulus_library/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def run_matching_table_builder(
self.cursor,
self.schema_name,
table_builder_name,
self.verbose,
verbose=self.verbose,
parser=self.db.parser(),
config=config,
)
Expand Down Expand Up @@ -348,7 +348,7 @@ def run_cli(args: dict):
db=databases.create_db_backend(args),
force_upload=args.get("replace_existing", False),
stats_build=args.get("stats_build", False),
umls_key=args.get("umls"),
umls_key=args.get("umls_key"),
)
try:
runner = StudyRunner(config.db, data_path=args.get("data_path"))
Expand All @@ -365,7 +365,7 @@ def run_cli(args: dict):
f"{target} was not found in available studies: "
f"{list(study_dict.keys())}.\n\n"
"If you are trying to run a custom study, make sure "
"you include `-s path/to/study/dir` as an arugment."
"you include `-s path/to/study/dir` as an argument."
)
if args["action"] == "clean":
runner.clean_study(
Expand All @@ -381,7 +381,7 @@ def run_cli(args: dict):
for target in args["target"]:
if args["builder"]:
runner.run_matching_table_builder(
study_dict[target], config=config
study_dict[target], args["builder"], config=config
)
else:
runner.clean_and_build_study(
Expand Down Expand Up @@ -452,7 +452,7 @@ def main(cli_args=None):
("region", "CUMULUS_LIBRARY_REGION"),
("schema_name", "CUMULUS_LIBRARY_DATABASE"),
("study_dir", "CUMULUS_LIBRARY_STUDY_DIR"),
("umls", "UMLS_API_KEY"),
("umls_key", "UMLS_API_KEY"),
("url", "CUMULUS_AGGREGATOR_URL"),
("user", "CUMULUS_AGGREGATOR_USER"),
("workgroup", "CUMULUS_LIBRARY_WORKGROUP"),
Expand Down
60 changes: 34 additions & 26 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ def test_count_builder_mapping(mock_path, tmp_path):
"study_python_counts_valid",
"-s",
"./tests/test_data",
"--database",
"test",
],
tmp_path,
)
Expand Down Expand Up @@ -159,8 +157,6 @@ def test_generate_sql(mock_path, tmp_path):
"study_python_valid",
"-s",
f"{tmp_path}",
"--database",
"test",
],
tmp_path,
)
Expand Down Expand Up @@ -195,8 +191,6 @@ def test_generate_md(mock_path, tmp_path):
"study_python_valid",
"-s",
f"{tmp_path}",
"--database",
"test",
],
tmp_path,
)
Expand All @@ -209,8 +203,6 @@ def test_generate_md(mock_path, tmp_path):
"study_python_valid",
"-s",
f"{tmp_path}",
"--database",
"test",
],
tmp_path,
)
Expand Down Expand Up @@ -263,20 +255,10 @@ def test_generate_md(mock_path, tmp_path):
)
def test_clean(mock_path, tmp_path, args, expected): # pylint: disable=unused-argument
mock_path.return_value = f"{Path(__file__).resolve().parents[0]}/test_data/"
cli.main(
cli_args=duckdb_args(["build", "-t", "core", "--database", "test"], tmp_path)
)
cli.main(cli_args=duckdb_args(["build", "-t", "core"], tmp_path))
with does_not_raise():
with mock.patch.object(builtins, "input", lambda _: "y"):
cli.main(
cli_args=[
*args,
"--db-type",
"duckdb",
"--database",
f"{tmp_path}/duck.db",
]
)
cli.main(cli_args=duckdb_args(args, tmp_path))
db = DuckDatabaseBackend(f"{tmp_path}/duck.db")
for table in db.cursor().execute("show tables").fetchall():
assert expected not in table
Expand Down Expand Up @@ -403,7 +385,7 @@ def test_cli_export_archive(tmp_path, args, input_txt, expects_files, raises):
def test_cli_transactions(tmp_path, study, finishes, raises):
with raises:
args = duckdb_args(
["build", "-t", study, "--database", "test", "-s", "tests/test_data/"],
["build", "-t", study, "-s", "tests/test_data/"],
f"{tmp_path}",
)
print(args[-1:])
Expand Down Expand Up @@ -443,11 +425,7 @@ def test_cli_stats_rebuild(tmp_path):
- that a results table is created when we explicitly ask for one with a CLI flag
"""

cli.main(
cli_args=duckdb_args(
["build", "-t", "core", "--database", "test"], tmp_path, stats=True
)
)
cli.main(cli_args=duckdb_args(["build", "-t", "core"], tmp_path, stats=True))
arg_list = [
"build",
"-s",
Expand Down Expand Up @@ -554,3 +532,33 @@ def test_cli_upload_filter(mock_upload_data, mock_glob, args, calls):
# filepath is in the third argument position in the upload data arg list
assert target in str(mock_upload_data.call_args[0][2])
assert mock_upload_data.call_count == calls


@pytest.mark.parametrize("mode", ["cli", "env"])
@mock.patch.dict(os.environ, clear=True)
# early exit with a dumb error
@mock.patch("cumulus_library.base_utils.StudyConfig", side_effect=ZeroDivisionError)
def test_cli_umls_parsing(mock_config, mode, tmp_path):
with pytest.raises(ZeroDivisionError):
match mode:
case "cli":
cli.main(cli_args=duckdb_args(["build", "--umls-key=MY-KEY"], tmp_path))
case "env":
with mock.patch.dict(os.environ, {"UMLS_API_KEY": "MY-KEY"}):
cli.main(cli_args=duckdb_args(["build"], tmp_path))

assert mock_config.call_args[1]["umls_key"] == "MY-KEY"


@mock.patch.dict(os.environ, clear=True)
def test_cli_single_builder(tmp_path):
cli.main(
cli_args=duckdb_args(["build", "--builder=patient", "--target=core"], tmp_path)
)
db = DuckDatabaseBackend(f"{tmp_path}/duck.db")
tables = {x[0] for x in db.cursor().execute("show tables").fetchall()}
assert {
"core__patient",
"core__patient_ext_ethnicity",
"core__patient_ext_race",
} == tables

0 comments on commit 6866553

Please sign in to comment.