From c14a746ba6446f8dd9b88ba8dc33419a7321c81e Mon Sep 17 00:00:00 2001
From: Kelvin Rodriguez <krodriguez@usgs.gov>
Date: Wed, 17 Aug 2022 11:32:43 -0700
Subject: [PATCH] downloadIsisData Script fixes (#5025)

* fixes for downloadIsisData

* added changelog line

* no more conda_prefix

* tweaked do string

* removed prints

* made usgs sources download second

* isis_data to usgs_data

* update readme with correc paths to .conf and script
---
 CHANGELOG.md                  |  2 ++
 README.md                     | 11 ++++---
 isis/CMakeLists.txt           |  2 +-
 isis/config/rclone.conf       | 62 +++++++++++++++++------------------
 isis/scripts/downloadIsisData | 45 ++++++++++++-------------
 5 files changed, 63 insertions(+), 59 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d3ebce9b46..d1b89169be 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -43,6 +43,8 @@ release.
 
 ### Fixed
 
+- Fixed bugs in downloadIsisData script [#5024](https://github.com/USGS-Astrogeology/ISIS3/issues/5024) 
+
 ## [7.1.0] - 2022-07-27
 
 ### Changed
diff --git a/README.md b/README.md
index ae6e1733d7..13324ead57 100644
--- a/README.md
+++ b/README.md
@@ -389,7 +389,7 @@ If you plan to work with data from all missions, then the download will require
 the outdated rsync download information can be found [here](https://github.com/USGS-Astrogeology/ISIS3/wiki/Outdated-ISIS-Data-Information)
 
 
-The ISIS Data Area is hosted on a combination of AWS S3 buckets and public http servers e.g. NAIF, Jaxa, ESA and not through conda channels like the ISIS binaries. This requires using the `downloadIsisData.py` script from within a terminal window within your Unix distribution, or from within WSL if running Windows 10. Downloading all mission data requires over 520 GB of disk space. If you want to acquire only certain mission data [click here](#Mission-Specific-Data-Downloads). To download all ISIS data files, continue reading.
+The ISIS Data Area is hosted on a combination of AWS S3 buckets and public http servers e.g. NAIF, Jaxa, ESA and not through conda channels like the ISIS binaries. This requires using the `downloadIsisData` script from within a terminal window within your Unix distribution, or from within WSL if running Windows 10. Downloading all mission data requires over 520 GB of disk space. If you want to acquire only certain mission data [click here](#Mission-Specific-Data-Downloads). To download all ISIS data files, continue reading.
 
 To download all ISIS data, use the following command:
 
@@ -513,11 +513,12 @@ You can download the script and config file from the repo:
 conda install -c conda-forge rclone
 
 # download the script and rclone config file
-curl https://github.com/USGS-Astrogeology/ISIS3/raw/dev/isis/scripts/downloadIsisData.py -o downloadIsisData.py
+curl https://raw.githubusercontent.com/USGS-Astrogeology/ISIS3/dev/isis/scripts/downloadIsisData -o downloadIsisData.py
 
-curl https://github.com/USGS-Astrogeology/ISIS3/raw/dev/isis/config/rclone.conf -o rclone.conf
+curl https://raw.githubusercontent.com/USGS-Astrogeology/ISIS3/dev/isis/config/rclone.conf -o rclone.conf
 
 # run the script as normal, using --config to point to where you downloaded the config file 
-python downloadIsisData.py sync <mission> $ISISDATA --config rclone.conf
-
+python downloadIsisData <mission> $ISISDATA --config rclone.conf
 ```
+
+> The script does not support python2, sometimes you need to explicitly use python3 with `python3 downloadIsisData <mission> $ISISDATA --config rclone.conf` 
diff --git a/isis/CMakeLists.txt b/isis/CMakeLists.txt
index 040eabe499..e8b252a038 100644
--- a/isis/CMakeLists.txt
+++ b/isis/CMakeLists.txt
@@ -610,7 +610,7 @@ install(FILES     ${CMAKE_BINARY_DIR}/version         DESTINATION  ${CMAKE_INSTA
 install(DIRECTORY ${CMAKE_SOURCE_DIR}/scripts         DESTINATION  ${CMAKE_INSTALL_PREFIX})
 install(PROGRAMS ${CMAKE_BINARY_DIR}/lib/Camera.plugin DESTINATION ${CMAKE_INSTALL_PREFIX}/lib/)
 install(PROGRAMS ${CMAKE_SOURCE_DIR}/scripts/downloadIsisData DESTINATION ${CMAKE_INSTALL_PREFIX}/bin/)
-install(PROGRAMS ${CMAKE_SOURCE_DIR}/config/rclone.conf DESTINATION ${CMAKE_INSTALL_PREFIX}/etc/isis/)
+install(FILES ${CMAKE_SOURCE_DIR}/config/rclone.conf DESTINATION ${CMAKE_INSTALL_PREFIX}/etc/isis/)
 
 # Trigger all post-install behavior.
 # - The only way to run commands post-install in CMake is to add a subdirectory at
diff --git a/isis/config/rclone.conf b/isis/config/rclone.conf
index 0b86075c79..50421d3f79 100644
--- a/isis/config/rclone.conf
+++ b/isis/config/rclone.conf
@@ -18,19 +18,19 @@ url = http://www.darts.isas.jaxa.jp/
 
 [apollo15_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/apollo15
+remote = asc_s3:asc-isisdata/usgs_data/apollo15
 
 [apollo16_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/apollo16
+remote = asc_s3:asc-isisdata/usgs_data/apollo16
 
 [apollo17_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/apollo17
+remote = asc_s3:asc-isisdata/usgs_data/apollo17
 
 [lro_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/lro
+remote = asc_s3:asc-isisdata/usgs_data/lro
 
 [tgo_naifKernels]
 type = alias
@@ -38,7 +38,7 @@ remote = esa:/data/SPICE/ExoMars2016/kernels/
 
 [tgo_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/tgo
+remote = asc_s3:asc-isisdata/usgs_data/tgo
 
 [dawn_naifKernels]
 type = alias
@@ -46,7 +46,7 @@ remote = naif:/pub/naif/DAWN/kernels
 
 [dawn_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/dawn
+remote = asc_s3:asc-isisdata/usgs_data/dawn
 
 [cassini_naifKernels]
 type = alias
@@ -54,7 +54,7 @@ remote = naif:/pub/naif/CASSINI/kernels
 
 [cassini_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/cassini
+remote = asc_s3:asc-isisdata/usgs_data/cassini
 
 [hayabusa2_naifKernels]
 type = alias
@@ -62,7 +62,7 @@ remote = jaxa:/pub/hayabusa2/spice_bundle/spice_kernels
 
 [hayabusa2_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/hayabusa2
+remote = asc_s3:asc-isisdata/usgs_data/hayabusa2
 
 [juno_naifKernels]
 type = alias
@@ -70,7 +70,7 @@ remote = naif:/pub/naif/pds/data/jno-j_e_ss-spice-6-v1.0/jnosp_1000/data/
 
 [juno_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/juno
+remote = asc_s3:asc-isisdata/usgs_data/juno
 
 [odyssey_naifKernels]
 type = alias
@@ -78,7 +78,7 @@ remote = naif:/pub/naif/M01/kernels
 
 [odyssey_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/odyssey
+remote = asc_s3:asc-isisdata/usgs_data/odyssey
 
 [mro_naifKernels]
 type = alias
@@ -86,7 +86,7 @@ remote = naif:/pub/naif/MRO/kernels
 
 [mro_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/mro
+remote = asc_s3:asc-isisdata/usgs_data/mro
 
 [mex_naifKernels]
 type = alias
@@ -94,11 +94,11 @@ remote = naif:/pub/naif/MEX/kernels
 
 [mex_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/mex
+remote = asc_s3:asc-isisdata/usgs_data/mex
 
 [legacybase_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/legacy_base
+remote = asc_s3:asc-isisdata/usgs_data/legacy_base
 
 [hayabusa_naifKernels]
 type = alias
@@ -106,7 +106,7 @@ remote = jaxa:/pub/spice/HAYABUSA/kernels
 
 [hayabusa_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/hayabusa
+remote = asc_s3:asc-isisdata/usgs_data/hayabusa
 
 [chandrayaan1_naifKernels]
 type = alias
@@ -114,7 +114,7 @@ remote = esa:/data/SPICE/CHANDRAYAAN-1/kernels
 
 [chandrayaan1_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/chandrayaan1
+remote = asc_s3:asc-isisdata/usgs_data/chandrayaan1
 
 [clementine1_naifKernels]
 type = alias
@@ -122,7 +122,7 @@ remote = naif:/pub/naif/pds/data/clem1-l-spice-6-v1.0/clsp_1000/data/
 
 [clementine1_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/clementine1
+remote = asc_s3:asc-isisdata/usgs_data/clementine1
 
 [kaguya_naifKernels]
 type = alias
@@ -130,7 +130,7 @@ remote = jaxa:/pub/pds3/sln-l-spice-6-v1.0/slnsp_1000/data/
 
 [kaguya_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/kaguya
+remote = asc_s3:asc-isisdata/usgs_data/kaguya
 
 [mariner10_naifKernels]
 type = alias
@@ -138,7 +138,7 @@ remote = naif:/pub/naif/M10/kernels
 
 [mariner10_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/mariner10
+remote = asc_s3:asc-isisdata/usgs_data/mariner10
 
 [messenger_naifKernels]
 type = alias
@@ -146,7 +146,7 @@ remote = naif:/pub/naif/pds/data/mess-e_v_h-spice-6-v1.0/messsp_1000/data/
 
 [messenger_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/messenger
+remote = asc_s3:asc-isisdata/usgs_data/messenger
 
 [mgs_naifKernels]
 type = alias
@@ -154,7 +154,7 @@ remote = naif:/pub/naif/pds/data/mgs-m-spice-6-v1.0/mgsp_1000/data/
 
 [mgs_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/mgs
+remote = asc_s3:asc-isisdata/usgs_data/mgs
 
 [near_naifKernels]
 type = alias
@@ -162,7 +162,7 @@ remote = naif:/pub/naif/pds/data/near-a-spice-6-v1.0/nearsp_1000/data/
 
 [near_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/near
+remote = asc_s3:asc-isisdata/usgs_data/near
 
 [newhorizons_naifKernels]
 type = alias
@@ -170,7 +170,7 @@ remote = naif:/pub/naif/pds/data/nh-j_p_ss-spice-6-v1.0/nhsp_1000/data/
 
 [newhorizons_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/newhorizons
+remote = asc_s3:asc-isisdata/usgs_data/newhorizons
 
 [osirisrex_naifKernels]
 type = alias
@@ -178,11 +178,11 @@ remote = naif:/pub/naif/pds/pds4/orex/orex_spice/spice_kernels/
 
 [osirisrex_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/osirisrex/
+remote = asc_s3:asc-isisdata/usgs_data/osirisrex/
 
 [rolo_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/rolo
+remote = asc_s3:asc-isisdata/usgs_data/rolo
 
 [rosetta_naifKernels]
 type = alias
@@ -190,7 +190,7 @@ remote = naif:/pub/naif/ROSETTA/kernels
 
 [rosetta_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/rosetta
+remote = asc_s3:asc-isisdata/usgs_data/rosetta
 
 [smart1_naifKernels]
 type = alias
@@ -198,7 +198,7 @@ remote = esa:/data/SPICE/SMART-1/kernels
 
 [smart1_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/smart1
+remote = asc_s3:asc-isisdata/usgs_data/smart1
 
 [viking1_naifKernels]
 type = alias
@@ -206,7 +206,7 @@ remote = naif:/pub/naif/VIKING/kernels
 
 [viking1_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/viking1
+remote = asc_s3:asc-isisdata/usgs_data/viking1
 
 [viking2_naifKernels]
 type = alias
@@ -214,7 +214,7 @@ remote = naif:/pub/naif/VIKING/kernels
 
 [viking2_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/viking2
+remote = asc_s3:asc-isisdata/usgs_data/viking2
 
 [voyager1_naifKernels]
 type = alias
@@ -222,7 +222,7 @@ remote = naif:/pub/naif/VOYAGER/kernels
 
 [voyager1_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/voyager1
+remote = asc_s3:asc-isisdata/usgs_data/voyager1
 
 [voyager2_naifKernels]
 type = alias
@@ -230,9 +230,9 @@ remote = naif:/pub/naif/VOYAGER/kernels
 
 [voyager2_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/voyager2
+remote = asc_s3:asc-isisdata/usgs_data/voyager2
 
 [base_usgs]
 type = alias
-remote = asc_s3:asc-isisdata/isis_data/base
+remote = asc_s3:asc-isisdata/usgs_data/base
 
diff --git a/isis/scripts/downloadIsisData b/isis/scripts/downloadIsisData
index b3a4ca1ee2..07b773dcb5 100755
--- a/isis/scripts/downloadIsisData
+++ b/isis/scripts/downloadIsisData
@@ -2,7 +2,6 @@
 
 from multiprocessing import ProcessError
 from multiprocessing.dummy import Process
-from pprint import pprint
 import logging as log
 import os
 import json
@@ -11,22 +10,22 @@ import subprocess
 import tempfile
 from shutil import which
 from os import path
-
+from collections import OrderedDict
 
 def find_conf():
     from pathlib import Path
     local_path = Path("rclone.conf")
-    install_path = Path(os.environ.get("CONDA_PREFIX")) / "etc" / "isis" / local_path
-    repo_path = Path(os.path.dirname(__file__)) / '..' / 'config' / 'rclone.conf' 
-    
+    # this should be installed in scripts folder, so config is one directory up in etc 
+    install_path = Path(os.path.dirname(__file__)) / '..' / "etc" / "isis" / 'rclone.conf'
+    repo_path = Path(os.path.dirname(__file__)) / '..' / 'config' / 'rclone.conf'
     if local_path.exists():
-        return str(local_path) 
+        return str(local_path)
     elif repo_path.exists():
         return str(repo_path)
     elif install_path.exists():
         return str(install_path)
     else:
-        return ""
+        raise RuntimeError("No config found, try passing it in explicitly with --config <path>")
 
 
 # set log level to debug
@@ -102,7 +101,9 @@ def create_rclone_arguments(destination, mission_name, dry_run=False, ntransfers
     """
     log.debug(f"Creating RClone command for {mission_name}")
     mission_dir_name, source_type = mission_name.replace(":", "").split("_")
-
+    if (mission_dir_name == "legacybase"):
+        # We still want things to go into base
+        mission_dir_name = "base"
     log.debug(f"Mission_dir_name: {mission_dir_name}, source_type: {source_type}")
 
     destination = os.path.join(destination, str(mission_dir_name).replace(":",""))
@@ -111,11 +112,10 @@ def create_rclone_arguments(destination, mission_name, dry_run=False, ntransfers
 
     if not os.path.exists(destination) and dry_run==False:
         log.debug("{destination} does not exist, making directory")
-        print(destination)
         os.makedirs(destination)
 
     extra_args=[f"{mission_name}",f"{destination}", "--progress", f"--checkers={ntransfers}", f"--transfers={ntransfers}", "--track-renames", f"--log-level={log.getLevelName(log.getLogger().getEffectiveLevel())}"]
-    
+
     if dry_run:
         extra_args.append("--dry-run")
 
@@ -139,25 +139,25 @@ def main(mission, dest, cfg_path, dry_run, ntransfers):
     log.debug(f"Using config: {cfg_path}")
     result = rclone("listremotes", config=cfg_path)
     config_sources = result.get('out').decode("utf-8").split("\n")
-    if config_sources == ['']: 
+    if config_sources == ['']:
         log.error("Remote sources came back empty. Get more info by re-running with verbose flag.")
         quit(-1)
     log.debug(f"Remote Sources: {config_sources}")
 
-    supported_missions = {}
-    for source in config_sources: 
+    supported_missions = OrderedDict({})
+    for source in sorted(config_sources, key=lambda x: x.split("_")[-1]):
         parsed_name = source.split("_")
         # If it is a mission, it should be in the format <mission_nam>_<source_type>
-        if len(parsed_name) == 2 and parsed_name[1] in ["usgs:", "naifKernels:"]: 
+        if len(parsed_name) == 2 and parsed_name[1] in ["usgs:", "naifKernels:"]:
             remotes_mission_name = parsed_name[0]
             supported_missions[remotes_mission_name] = supported_missions.get(remotes_mission_name, []) + [source]
 
     log.debug(f"Supported missions:\n {supported_missions.keys()}")
     log.debug(f"Complete Dictionary:\n {json.dumps(supported_missions, indent=2)}")
 
-    if not mission in supported_missions.keys():
+    if not mission in supported_missions.keys() and mission.upper() not in ("ALL", "LEGACYBASE"):
         raise LookupError(f"{mission} is not in the list of supported missions: {supported_missions.keys()}")
- 
+
     if mission == "legacybase":
         args = create_rclone_arguments(dest, "legacybase_usgs:", dry_run, ntransfers)
         rclone(command="sync", extra_args=args, config=cfg_path, redirect_stdout=False, redirect_stderr=False)
@@ -170,16 +170,15 @@ def main(mission, dest, cfg_path, dry_run, ntransfers):
     else:
         for remote in supported_missions[mission]:
             args = create_rclone_arguments(dest, remote, dry_run, ntransfers)
-            rclone(command="sync", extra_args=args, config=cfg_path, redirect_stdout=False, redirect_stderr=False) 
+            rclone(command="sync", extra_args=args, config=cfg_path, redirect_stdout=False, redirect_stderr=False)
 
 
-if __name__ == '__main__': 
+if __name__ == '__main__':
     helpString = (
     '''This will allow for a user to download isis data directly to their machine from the USGS S3 buckets as well as public end points\n\n'''
-    '''To use the download ISIS Data script you must supply 3 parameters with an optional 4th.\n\n'''
-    '''<rclone command> <Mission name> <destination to copy to> <--dry-run (optional)> \n'''
-    '''Example of how to run this program:\n'''
-    '''python downloadIsisData.py tgo ~/isisData/tgo\n\n'''
+    '''<Mission name> <destination to copy to> <--dry-run (optional)> \n\n'''
+    '''Example of how to run this program:\n\n'''
+    '''    downloadIsisData tgo ~/isisData/\n\n'''
     '''NOTE: if you would like to download the data for every mission supply the value ALL for the <Mission name> parameter''')
     parser = argparse.ArgumentParser(description = helpString, formatter_class=argparse.RawTextHelpFormatter)
     parser.add_argument('mission', help='mission for files to be downloaded')
@@ -206,3 +205,5 @@ if __name__ == '__main__':
     log.basicConfig(**log_kwargs)
 
     main(args.mission, args.dest, os.path.expanduser(args.config), args.dry_run, args.num_transfers)
+
+
-- 
GitLab