From 411b87b6d9cd9aba36211ed28205c7f13b4186c8 Mon Sep 17 00:00:00 2001 From: "Janet L. Goldstein" <jgoldste@nrao.edu> Date: Tue, 13 Apr 2021 09:35:29 -0600 Subject: [PATCH] WS-179: subtask #6 -- remove argparse namespace dependencies in classes * args are accessed only in locations_report.__init()__ * directory/file-related errors throw custom exception --- .../datafetcher/file_retrievers.py | 2 +- .../datafetcher/locations_report.py | 32 +++++++++---------- .../datafetcher/test/test_df_function.py | 16 +++------- 3 files changed, 22 insertions(+), 28 deletions(-) diff --git a/apps/cli/executables/datafetcher/datafetcher/file_retrievers.py b/apps/cli/executables/datafetcher/datafetcher/file_retrievers.py index a509e19ae..b61c162be 100644 --- a/apps/cli/executables/datafetcher/datafetcher/file_retrievers.py +++ b/apps/cli/executables/datafetcher/datafetcher/file_retrievers.py @@ -52,7 +52,7 @@ class NGASFileRetriever: download_url = "http://" + server + "/RETRIEVE" destination = self._get_destination(file_spec) if destination.exists() and not self.force_overwrite and not self.dry_run: - raise FileExistsError(f"{destination} exists; aborting") + raise FileErrorException(f"{destination} exists; aborting") self._make_basedir(destination) diff --git a/apps/cli/executables/datafetcher/datafetcher/locations_report.py b/apps/cli/executables/datafetcher/datafetcher/locations_report.py index f8e751ee6..e46655a49 100644 --- a/apps/cli/executables/datafetcher/datafetcher/locations_report.py +++ b/apps/cli/executables/datafetcher/datafetcher/locations_report.py @@ -34,22 +34,19 @@ class LocationsReport: """ Builds a location report """ def __init__(self, args: Namespace, settings: Dict): - - try: - self.verbose = args.verbose or False - except AttributeError: - # doesn't matter; verbose is going away soon - self.verbose = False - - self._capture_and_validate_input(args, settings) - self._run() - - def _capture_and_validate_input(self, args, settings): if args is None: raise MissingSettingsException( "arguments (locator and/or report file, destination) are required" ) - self.args = args + self.product_locator = args.product_locator + self.location_file = args.location_file + if not self.product_locator and not self.location_file: + raise NoLocatorException("either product locator or report file must be specified") + if self.product_locator and self.location_file: + raise NoLocatorException( + "either product locator -or- report file must be specified -- not both" + ) + if settings is None: raise MissingSettingsException("CAPO settings are required") self.settings = settings @@ -57,10 +54,13 @@ class LocationsReport: if not self.settings["execution_site"]: raise MissingSettingsException("execution_site is required") - self.product_locator = args.product_locator - self.location_file = args.location_file - if not self.product_locator and not self.location_file: - raise NoLocatorException("either product locator or report file must be specified") + try: + self.verbose = args.verbose or False + except AttributeError: + # doesn't matter; verbose is going away soon + self.verbose = False + + self._run() def _run(self): self.files_report = self._get_files_report() diff --git a/apps/cli/executables/datafetcher/test/test_df_function.py b/apps/cli/executables/datafetcher/test/test_df_function.py index 1f337682f..5a7035486 100644 --- a/apps/cli/executables/datafetcher/test/test_df_function.py +++ b/apps/cli/executables/datafetcher/test/test_df_function.py @@ -7,8 +7,8 @@ import pytest from datafetcher.errors import ( NGASFetchError, - MissingSettingsException, NGASServiceErrorException, + FileErrorException, ) # pylint: disable=C0115, C0116, C0200, C0415, R0801, R0902, R0903, R0914, R1721, @@ -163,21 +163,15 @@ def test_no_overwrite_without_force(make_tempdir, capo_settings): str(top_level), ] - with pytest.raises(FileExistsError): + with pytest.raises(FileErrorException): try_to_launch_df(capo_settings, args) retrieved = [ file for file in top_level.rglob("*") if file.is_file() and not str(file).endswith(".json") ] - num_not_too_big_files_expected = 28 - assert len(retrieved) == num_not_too_big_files_expected # get rid of all the files we downloaded, plus the log - deleted = [ - file.unlink() for file in retrieved if file.is_file() and not str(file).endswith(".json") - ] - - assert len(deleted) >= num_not_too_big_files_expected + [file.unlink() for file in retrieved if file.is_file() and not str(file).endswith(".json")] @pytest.mark.skipif(not RUN_ALL, reason="debug") @@ -257,7 +251,7 @@ def test_dies_with_bad_server_info(make_tempdir, settings): @pytest.mark.skipif(not RUN_ALL, reason="debug") -def test_missing_setting_exc_on_bad_destination(settings): +def test_throws_file_err_for_existing_destination(settings): args = [ "--profile", TEST_PROFILE, @@ -266,7 +260,7 @@ def test_missing_setting_exc_on_bad_destination(settings): "--output-dir", "floob", ] - with pytest.raises(MissingSettingsException): + with pytest.raises(FileErrorException): launch_datafetcher(args, settings.capo_settings) -- GitLab