SSA-6324: using Pathlib instead of os where possible. added a little more documentation.

apps/cli/executables/datafetcher/src/datafetcher/file_retrievers.py

@@ -10,7 +10,6 @@ from pathlib import Path
 
 import requests
 from bs4 import BeautifulSoup
-import lxml
 
 from .errors import SizeMismatchException, \
     NGASServiceErrorException, \
@@ -46,14 +45,11 @@ class NGASFileRetriever:
         """
         download_url = 'http://' + server + '/RETRIEVE'
         destination = self._get_destination(file_spec)
-
-        if os.path.exists(
-                destination) and not self.force_overwrite and not self.dry_run:
+        if Path.exists(destination) and not self.force_overwrite and not self.dry_run:
             raise FileExistsError(f'{destination} exists; aborting')
 
         self._make_basedir(destination)
 
-
         func = self._copying_fetch if retrieve_method == RetrievalMode.COPY \
             else self._streaming_fetch
         retryer = Retryer(func, MAX_TRIES, SLEEP_INTERVAL_SECONDS, self._LOG)
@@ -75,10 +71,10 @@ class NGASFileRetriever:
         """
 
         try:
+            output_dir = Path(self.output_dir)
             if file_spec['subdirectory'] is None:
-                return os.path.join(self.output_dir, file_spec['relative_path'])
-            return os.path.join(self.output_dir, file_spec['subdirectory'],
-                                file_spec['relative_path'])
+                return output_dir / file_spec['relative_path']
+            return output_dir / file_spec['subdirectory'] / file_spec['relative_path']
         except KeyError as k_err:
             raise MissingSettingsException(k_err)
 
@@ -93,8 +89,8 @@ class NGASFileRetriever:
         :return:
         """
         if not self.dry_run:
-            basedir = os.path.dirname(destination)
-            if os.path.isdir(basedir) and not os.access(basedir, os.W_OK):
+            basedir = Path(destination).parent
+            if Path.is_dir(basedir) and not os.access(basedir, os.W_OK):
                 raise FileErrorException(
                     f'output directory {basedir} is not writable')
             try:
@@ -114,7 +110,7 @@ class NGASFileRetriever:
         """
         if not self.dry_run:
             self._LOG.debug(f'verifying fetch of {destination}')
-            if not os.path.exists(destination):
+            if not Path.exists(destination):
                 raise NGASServiceErrorException(
                     f'file not delivered to {destination}')
             if file_spec['size'] != os.path.getsize(destination):
@@ -135,7 +131,7 @@ class NGASFileRetriever:
 
         params = {'file_id': file_spec['ngas_file_id'],
                   'processing': _DIRECT_COPY_PLUGIN,
-                  'processingPars': 'outfile=' + destination,
+                  'processingPars': 'outfile=' + str(destination),
                   'file_version': file_spec['version']}
         self._LOG.debug('attempting copying download:\nurl: {}\ndestination: {}'
                         .format(download_url, destination))
@@ -195,11 +191,11 @@ class NGASFileRetriever:
                     actual_size = os.path.getsize(destination)
                     if actual_size == 0:
                         raise FileErrorException(
-                            f'{os.path.basename(destination)} '
+                            f'{Path(destination).name} '
                             f'was not retrieved')
                     if actual_size != expected_size:
                         raise SizeMismatchException(
-                            f'expected {os.path.basename(destination)} '
+                            f'expected {Path(destination).name} '
                             f'to be {expected_size} bytes; '
                             f'was {actual_size} bytes'
                         )

apps/cli/executables/datafetcher/src/datafetcher/product_fetchers.py

@@ -6,6 +6,7 @@ import copy
 import os
 from argparse import Namespace
 from concurrent.futures import ThreadPoolExecutor, as_completed
+from pathlib import Path
 from typing import Dict
 
 from .errors import NGASServiceErrorException
@@ -152,8 +153,7 @@ class ParallelProductFetcher(BaseProductFetcher):
                     if dirnames:
                         # we can expect one subdir: the external_name associated
                         # with the product locator
-                        subdir = dirnames[0]
-                        to_walk = os.path.join(dirname, subdir)
+                        to_walk = Path(dirname) / dirnames[0]
                         for dname, dnames, files in os.walk(to_walk):
                             if self.num_files_expected <= len(files):
                                 self.num_files_retrieved += len(files)

apps/cli/executables/datafetcher/src/datafetcher/utilities.py

@@ -51,7 +51,7 @@ def path_is_accessible(path):
     ''' Is this path readable, executable, and writable?
     '''
     can_access = os.access(path, os.F_OK)
-    can_access = can_access and os.path.isdir(path)
+    can_access = can_access and pathlib.Path.is_dir(path)
     can_access = can_access and os.access(path, os.R_OK)
     can_access = can_access and os.access(path, os.W_OK)
     can_access = can_access and os.access(path, os.X_OK)
@@ -196,8 +196,9 @@ class FlexLogger():
             raise MissingSettingsException('class name is required')
         log_pathname = f'{class_name}_{str(time())}.log'
         try:
-            self.logfile = os.path.join(output_dir, log_pathname)
-            self.logger = logging.getLogger(self.logfile)
+
+            self.logfile = pathlib.Path(output_dir, log_pathname)
+            self.logger = logging.getLogger(str(self.logfile))
             self.verbose = verbose
             handler = logging.FileHandler(self.logfile)
             formatter = logging.Formatter(LOG_FORMAT)

apps/cli/executables/datafetcher/test/Dockerfile

 # datafetcher Dockerfile
 #
-# TO BUILD the docker image:
+# TO BUILD the docker image: -don't- "docker build" directly!
+# use docker_build.sh:
 # from apps/cli/executables/datafetcher,
-# docker build . -f test/Dockerfile -t datafetcher_test:N
+# ./docker_build,sh datafetcher_test:N
 # where '-t' specifies a name and N' is the version.
 # (If 'N' is omitted, version is 'latest' by default.)
 # tag is not required, but without it the container name is

apps/cli/executables/datafetcher/test/docker-build.sh

@@ -6,7 +6,8 @@
 # on boxes that can see /home, but which on boxes
 # that can't is likely to be at ~/home/.capo for
 # any given user. Find local.properties and
-# copy it to our test directory.
+# copy it to our test directory. Dockerfiles
+# do not support conditional logic; hence this script.
 
 FILENAME=local.properties
 CONTAINER_NAME=$1;shift
@@ -16,7 +17,10 @@ then
   exit 1
 fi
 
-
+# The preferred version of Capo .properties files is always
+# the one at /home/casa/capo, -if- this is visible
+# (i.e., NRAO internal system). If not (i.e., developer laptop),
+# get the one in the user's .capo directory
 if [ -e /home/casa/capo/${FILENAME} ]
 then
     SOURCE=/home/casa/capo/${FILENAME}

apps/cli/executables/datafetcher/test/testing_utils.py

@@ -7,12 +7,12 @@ import json
 import os
 from pathlib import Path
 
-import pytest
+from pycapo import CapoConfig
+
 from datafetcher.errors import MissingSettingsException, NoProfileException
 from datafetcher.locations_report import LocationsReport
 from datafetcher.utilities import REQUIRED_SETTINGS, get_arg_parser, \
     ExecutionSite
-from pycapo import CapoConfig
 
 TEST_PROFILE = 'local'
 
@@ -58,19 +58,19 @@ LOCATION_REPORTS = {
 }
 
 def get_test_data_dir():
-    here = os.path.abspath(os.curdir)
-    for root, dirnames, _ in os.walk(here):
+    """ where's our test data? """
+    for root, dirnames, _ in os.walk(Path.cwd()):
         if str(root).endswith('test'):
             for dirname in dirnames:
                 if dirname == 'data':
-                    return os.path.join(root, dirname)
+                    return Path(root, dirname)
     return None
 
 def get_locations_file(key: str):
     ''' return the location report file specified by key '''
     report_spec = LOCATION_REPORTS[key]
     filename = report_spec['filename']
-    return os.path.join(get_test_data_dir(), filename)
+    return Path(get_test_data_dir(), filename)
 
 def get_locations_report(key: str):
     ''' return the location report specified by key '''
@@ -130,7 +130,7 @@ def find_newest_fetch_log_file(target_dir: Path):
         for filename in filenames:
             if filename.startswith('DataFetcher') \
                     and filename.endswith('.log'):
-                logfiles.append(os.path.join(root, filename))
+                logfiles.append(Path(root, filename))
 
     if logfiles:
         return max(logfiles, key=os.path.getctime)
@@ -138,20 +138,19 @@ def find_newest_fetch_log_file(target_dir: Path):
     return None
 
 def get_test_capo_settings():
-    capo = CapoConfig(profile='local')
+    """ get the capo settings we'll need for the tests """
+    capo = CapoConfig(profile=TEST_PROFILE)
     result = dict()
     for setting in REQUIRED_SETTINGS:
-        value = None
         setting = setting.upper()
         try:
-            value = capo[setting]
+            result[REQUIRED_SETTINGS[setting]] = capo[setting]
         except KeyError:
             raise MissingSettingsException('missing required setting "{}"'
                                            .format(setting))
-        result[REQUIRED_SETTINGS[setting]] = value
 
     if result is None or len(result) == 0:
-        pytest.fail('Required Capo settings were not found')
+        raise MissingSettingsException('Required Capo settings were not found')
 
     for setting in result:
         print(f'{setting} = {result[setting]}')
@@ -163,12 +162,11 @@ def get_test_capo_settings():
 
     # be sure download location is accessible
     dl_loc = result['download_dir']
-    if not os.path.isdir('/lustre') and '/lustre' in dl_loc:
+    if not Path.is_dir(Path('/lustre')) and '/lustre' in dl_loc:
         result['download_dir'] = '/var/tmp/'
 
     return result
 
-
 def get_metadata_db_settings(profile):
     """ Get Capo settings needed to connect to archive DB
     :param profile:
@@ -176,14 +174,13 @@ def get_metadata_db_settings(profile):
     """
     result = dict()
     if profile is None:
-        raise NoProfileException('CAPO_PROFILE required, none provided')
-    capo = CapoConfig(profile='local', path=os.path.abspath('.'))
+        raise NoProfileException('CAPO_PROFILE required; none provided')
+    capo = CapoConfig(profile=TEST_PROFILE)
     fields = ['jdbcDriver', 'jdbcUrl', 'jdbcUsername', 'jdbcPassword']
     qualified_fields = ['metadataDatabase.' + field for field in fields]
     for field in qualified_fields:
         try:
-            value = capo[field]
-            result[field] = value
+            result[field] = capo.get(field)
         except KeyError:
             raise MissingSettingsException(
                 f'missing required setting "{field}"')