From 0bae86ab1066498c8ef6cf24eca3f78400f187e2 Mon Sep 17 00:00:00 2001 From: Brad Macdonald <52762200+BWMac@users.noreply.github.com> Date: Wed, 23 Oct 2024 12:15:11 -0600 Subject: [PATCH] [AG-1557] Fixes ModelAD `File size must be at least one byte` Error (#150) * updates argument and option descriptions * check for reports * adds test coverage * updates docs --- README.md | 2 +- src/agoradatatools/process.py | 13 +++++++++---- src/agoradatatools/reporter.py | 3 +-- tests/test_reporter.py | 12 +++++++++++- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ad34b762..3ca32f7a 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ Perform the following one-time steps to set up your local environment and obtain pipenv shell ``` -6. You can check if the package was isntalled correctly by running `adt --help` in the terminal. If it returns instructions about how to use the CLI, installation was successful and you can run the pipeline by providing the desired [config file](#config) as an argument. The following example command will execute the pipeline using ```test_config.yaml```: +6. You can check if the package was isntalled correctly by running `adt --help` in the terminal. If it returns instructions about how to use the CLI, installation was successful and you can run the pipeline by providing the desired [config file](#config) as an argument. Be sure to review these instructions prior to executing a processing run. The following example command will execute the pipeline using ```test_config.yaml```: ```bash adt test_config.yaml diff --git a/src/agoradatatools/process.py b/src/agoradatatools/process.py index 0b518e16..45329086 100644 --- a/src/agoradatatools/process.py +++ b/src/agoradatatools/process.py @@ -301,27 +301,32 @@ def process_all_files( app = Typer() -input_path_arg = Argument(..., help="Path to configuration file for processing run") +input_path_arg = Argument( + ..., help="Path to configuration file for processing run (Required)." +) platform_opt = Option( "LOCAL", "--platform", "-p", - help="Platform that is running the process. Must be one of LOCAL, GITHUB, or NEXTFLOW.", + help="Platform that is running the process. Must be one of LOCAL, GITHUB, or NEXTFLOW (Optional).", show_default=True, ) run_id_opt = Option( None, "--run_id", "-r", - help="Run ID of the process.", + help="Run ID of the process. This is used to identify the run in the GX table. (Optional)", show_default=True, ) upload_opt = Option( False, "--upload", "-u", - help="Toggles whether or not files will be uploaded to Synapse.", + help="Toggles whether or not files will be uploaded to Synapse. The absence of this option means " + "that neither output data files nor GX reports will be uploaded to Synapse. Setting " + "`--upload` in the command will cause both to be uploaded. This option is used to control " + "the upload behavior of the process.", show_default=True, ) synapse_auth_opt = Option( diff --git a/src/agoradatatools/reporter.py b/src/agoradatatools/reporter.py index c5a7dce5..fd321650 100644 --- a/src/agoradatatools/reporter.py +++ b/src/agoradatatools/reporter.py @@ -119,9 +119,8 @@ def _update_reports_before_upload(self) -> None: def update_table(self) -> None: """Updates the Synapse table adding one new row for each DatasetReport object if the platform is not LOCAL.""" - if self.platform != Platform.LOCAL: + if self.platform != Platform.LOCAL and self.reports: self._update_reports_before_upload() - self.syn.store( synapseclient.Table( self.table_id, diff --git a/tests/test_reporter.py b/tests/test_reporter.py index 4e9478aa..6994b99b 100644 --- a/tests/test_reporter.py +++ b/tests/test_reporter.py @@ -71,10 +71,11 @@ def test_update_reports_before_upload(self, mock_datetime): mock_datetime.datetime.now.return_value.strftime.assert_called_once() assert self.test_reporter.reports[0] == self.upload_report - def test_update_table(self, syn): + def test_update_table_platform_not_local_and_reports_not_empty(self, syn): with patch.object(syn, "store") as mock_store, patch.object( self.test_reporter, "_update_reports_before_upload" ) as mock_update_reports_before_upload: + self.test_reporter.reports = [self.test_report] self.test_reporter.update_table() mock_store.assert_called_once() @@ -88,3 +89,12 @@ def test_update_table_when_platform_is_local(self, syn): mock_store.assert_not_called() mock_update_reports_before_upload.assert_not_called() + + def test_update_table_platform_not_local_and_reports_empty(self, syn): + with patch.object(syn, "store") as mock_store, patch.object( + self.test_reporter, "_update_reports_before_upload" + ) as mock_update_reports_before_upload: + self.test_reporter.update_table() + + mock_store.assert_not_called() + mock_update_reports_before_upload.assert_not_called()