Change from optparse to argparse, part 3

.travis.yml

@@ -9,8 +9,8 @@ install:
   - python setup.py install
 script:
   - python setup.py test
-  - pycodestyle --ignore=E501,W503 resync bin tests
+  - pycodestyle --ignore=E501,W503 resync tests resync-sync resync-build resync-explorer
-  - pep257 resync bin tests
+  - pep257 resync bin tests resync-sync resync-build resync-explorer
   - coverage run --source=resync setup.py test
 after_success:
   - coveralls

CHANGES.md

@@ -3,6 +3,9 @@
 The current ResourceSync specification is standardized as ANSI/NISO Z39.99-2017 <http://www.openarchives.org/rs/1.1/toc>, the prior version was ANSI/NISO Z39.99-2014 <http://www.openarchives.org/rs/1.0/toc>.
 
 v2.0.0 ???
+  * Split old `resync` script into `resync-sync` and `resync-build`
+  * Move scripts from `bin` dir to base dir for easier testing/development
+  * Switch from optparse to argparse, use exclusive argument group for commands
   * Add --access_token option to pass bearer token with web requests
   * Add --delay option to pause between successive web requests
   * Drop Python 2.7, 3.3 & 3.4 from tests, add 3.7 & 3.8

resync-build

@@ -1,5 +1,5 @@
 #!/usr/bin/env python
-"""resync: The ResourceSync command line client.
+"""resync-build: The ResourceSync command line list builder.
 
 Copyright 2012-2020 Simeon Warner
 
@@ -33,28 +33,16 @@ def main():
 
     # Options and arguments
     parser = argparse.ArgumentParser(
-        description="ResourceSync command line client (v" + __version__ + ")\n\n"
+        description="ResourceSync build script (v" + __version__ + ")",
-                    "MODES - REMOTE and LOCAL - one must be specified\n"
-                    "These modes use a remote source that is specified in a "
-                    "set of uri=path mappings and potentially also using an "
-                    "explicit --sitemap location.",
         formatter_class=argparse.ArgumentDefaultsHelpFormatter)
 
     parser._optionals = parser.add_argument_group(
-       'MODES OF OPERATION (must specify one only). The REMOTE modes use a '
+        'MODES OF OPERATION (must specify one only). The REMOTE modes use a '
-       'remote source that is specified in a set of uri=path mappings and '
+        'remote source that is specified in a set of uri=path mappings and '
-       'potentially also using an explicit --sitemap location. The LOCAL '
+        'potentially also using an explicit --sitemap location. The LOCAL '
-       'modes operate only to create ResourceSync descriptions on the local '
+        'modes operate only to create ResourceSync descriptions on the local '
-       'filesystem based on local content')
+        'filesystem based on local content')
     rem = parser.add_mutually_exclusive_group(required=True)
-    rem.add_argument('--baseline', '-b', action='store_true',
-                     help='REMOTE: baseline sync of resources from remote source (src) to local filesystem (dst)')
-    rem.add_argument('--incremental', '--inc', '-i', action='store_true',
-                     help='REMOTE: incremental sync of resources from remote source (src) to local filesystem (dst). Uses either timestamp recorded from last baseline or incremental sync for this source, or explicit --from parameter, to determine the earlier update timestamp to act on.')
-    rem.add_argument('--audit', '-a', action='store_true',
-                     help="REMOTE: audit sync state of destination wrt source")
-    rem.add_argument('--parse', '-p', action='store_true',
-                     help="REMOTE: parse a remote sitemap/sitemapindex (from mapping or explicit --sitemap) and show summary information including document type and number of entries")
     rem.add_argument('--write-resourcelist', '--write-resource-list', action='store_true',
                      help="LOCAL: write a resource list based on files on disk using uri=path mappings "
                           "in reverse to calculate URIs from the local paths. Scans local disk "
@@ -83,82 +71,61 @@ def main():
                           "options as for --write-changelist")
 
     # Positional arguments
-    map=parser.add_argument_group('URI MAPPING TO FILESYSTEM for REMOTE modes')
+    map = parser.add_argument_group('URI MAPPING TO FILESYSTEM for REMOTE modes')
     map.add_argument(metavar='uri=path | uri path', dest='map', type=str, nargs='*',
                      help="remote URI of source for remote synchronization operations (may "
                           "also combine uri=local path)")
 
-    # Specification of map between remote URI and local file paths, and remote
-    # sitemap
     nam = parser.add_argument_group('FILE/URI NAMING OPTIONS')
     nam.add_argument('--outfile', type=str, action='store',
-                   help="write output to specified file rather than STDOUT or default")
+                     help="write output to specified file rather than STDOUT or default")
     nam.add_argument('--paths', type=str, action='store',
-                   help="explicit set of paths for disk scan --resourceslist or --changelist "
+                     help="explicit set of paths for disk scan --resourceslist or --changelist "
-                        "generation")
+                          "generation")
     nam.add_argument('--sitemap', type=str, action='store',
-                   help="explicitly set sitemap name, overriding default sitemap.xml "
+                     help="explicitly set sitemap name, overriding default sitemap.xml "
-                        "appended to first source URI specified in the mappings")
+                          "appended to first source URI specified in the mappings")
     nam.add_argument('--capabilitylist', '--capability-list', type=str, action='store',
-                   help="explicitly set capability list URI to search for instead of "
+                     help="explicitly set capability list URI to search for instead of "
-                        "looking for the source description")
+                          "looking for the source description")
     nam.add_argument('--reference', type=str, action='store',
-                   help="reference sitemap name for --write-changelist calculation")
+                     help="reference sitemap name for --write-changelist calculation")
     nam.add_argument('--newreference', type=str, action='store',
-                   help="updated reference sitemap name for --write-changelist calculation")
+                     help="updated reference sitemap name for --write-changelist calculation")
-    nam.add_argument('--changelist-uri', '--change-list-uri', type=str, action='store',
-                   help="explicitly set the changelist URI that will be use in --inc mode, "
-                        "overrides process of getting this from the sitemap")
 
     lks = parser.add_argument_group("LINK GENERATION")
     lks.add_argument('--link', type=str, action='append',
-                   help="add discovery links to the output sitemap, "
+                     help="add discovery links to the output sitemap, "
-                        "format: rel,href[,att1=val1,att2=val2] "
+                          "format: rel,href[,att1=val1,att2=val2] "
-                        "(repeat option for multiple links)")
+                          "(repeat option for multiple links)")
     lks.add_argument('--describedby-link', type=str, action='store',
-                   help="add an <rs:md rel=\"describedby\" link to "
+                     help="add an <rs:md rel=\"describedby\" link to "
-                        "a description of the feed at the URI given")
+                          "a description of the feed at the URI given")
     lks.add_argument('--sourcedescription-link', '--source-description-link',
-                   type=str, action='store',
+                     type=str, action='store',
-                   help="for a Capability List add a <rs:md rel=\"up\" link to the"
+                     help="for a Capability List add a <rs:md rel=\"up\" link to the"
-                        "Source Description document at the URI given, else ignored")
+                          "Source Description document at the URI given, else ignored")
     lks.add_argument('--capabilitylist-link', '--capability-list-link',
-                   type=str, action='store',
+                     type=str, action='store',
-                   help="for all documents except a Capability List or a "
+                     help="for all documents except a Capability List or a "
-                        "Source Description, add an <rs:md rel=\"up\" link "
+                          "Source Description, add an <rs:md rel=\"up\" link "
-                        "to the Capability List at the URI given")
+                          "to the Capability List at the URI given")
 
     # Options that apply to multiple modes
     opt = parser.add_argument_group('MISCELANEOUS OPTIONS')
     add_shared_misc_options(opt, default_logfile=DEFAULT_LOGFILE)
-    opt.add_argument('--delete', action='store_true',
-                     help="allow files on destination to be deleted")
     opt.add_argument('--empty', action='store_true',
                      help="combine with --changelist to write and empty changelist, perhaps with links")
-    opt.add_argument('--strictauth', action='store_true',
-                     help="use more strict checking of URLs to ensure that the ResourceSync "
-                          "documents refer only to resources on the same server or sub-domains, "
-                          "and on the same server to sub-paths. This is the authority model "
-                          "of Sitemaps but there are legitimate uses where these rules would "
-                          "not be followed.")
     opt.add_argument('--warc', action='store_true',
                      help="write dumps in WARC format (instead of ZIP+Sitemap default)")
     opt.add_argument('--dryrun', '-n', action='store_true',
                      help="don't update local resources, say what would be done")
-    opt.add_argument('--ignore-failures', action='store_true',
-                     help="continue past download failures")
     # These likely only useful for experimentation
     opt.add_argument('--max-sitemap-entries', type=int, action='store',
                      help="override default size limits")
     opt.add_argument('--eval', '-e', action='store_true',
                      help="output evaluation of source/client synchronization performance... "
                           "be warned, this is very verbose")
-    opt.add_argument('--tries', '-t', type=int, action='store', metavar='TRIES',
-                     help="set number of tries to TRIES. The default is to retry 20 times, "
-                          "with the exception of fatal errors like \"connection refused\" "
-                          "or \"not found\" (404), which are not retried.")
-    opt.add_argument('--timeout', '-T', type=int, action='store', metavar='SECONDS',
-                     help="set the request timeout for resource downloads to SECONDS seconds")
 
     args = parser.parse_args()
 
@@ -188,16 +155,8 @@ def main():
             c.allow_multifile = not args.multifile
         if (args.noauth):
             c.noauth = args.noauth
-        if (args.strictauth):
-            c.strictauth = args.strictauth
         if (args.max_sitemap_entries):
             c.max_sitemap_entries = args.max_sitemap_entries
-        if (args.ignore_failures):
-            c.ignore_failures = args.ignore_failures
-        if (args.tries):
-            c.tries = args.tries
-        if (args.timeout):
-            c.timeout = args.timeout
 
         # Links apply to anything that writes sitemaps
         links = parse_links(args.link)
@@ -216,16 +175,7 @@ def main():
                              'href': args.describedby_link})
 
         # Finally, do something...
-        if (args.baseline or args.audit):
+        if (args.write_resourcelist or args.write_resourcedump):
-            c.baseline_or_audit(allow_deletion=args.delete,
-                                audit_only=args.audit)
-        elif (args.incremental):
-            c.incremental(allow_deletion=args.delete,
-                          change_list_uri=args.changelist_uri,
-                          from_datetime=args.from_datetime)
-        elif (args.parse):
-            c.parse_document()
-        elif (args.write_resourcelist or args.write_resourcedump):
             c.write_resource_list(paths=args.paths,
                                   outfile=args.outfile,
                                   links=links,

resync-explorer

@@ -16,7 +16,7 @@ Copyright 2012-2020 Simeon Warner
    limitations under the License
 """
 
-import optparse
+import argparse
 import sys
 
 from resync import __version__
@@ -33,38 +33,38 @@ def main():
         sys.exit("This program requires python version 3.5 or later")
 
     # Options and arguments
-    p = optparse.OptionParser(description='ResourceSync explorer',
+    parser = argparse.ArgumentParser(
-                              usage='usage: %prog [options] uri_path local_path  (-h for help)',
+        description='ResourceSync explorer (v' + __version__ + ')',
-                              version='%prog ' + __version__)
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter)
 
     # Specification of map between remote URI and local file paths, and remote
     # sitemap
-    nam = p.add_option_group('FILE/URI NAMING OPTIONS')
+    nam = parser.add_argument_group('FILE/URI NAMING OPTIONS')
-    nam.add_option('--outfile', type=str, action='store',
+    nam.add_argument('--outfile', type=str, action='store',
-                   help="write sitemap to specified file rather than STDOUT")
+                     help="write sitemap to specified file rather than STDOUT")
-    nam.add_option('--paths', type=str, action='store',
+    nam.add_argument('--paths', type=str, action='store',
-                   help="explicit set of paths for disk scan --resourceslist or --changelist "
+                     help="explicit set of paths for disk scan --resourceslist or --changelist "
-                        "generation")
+                          "generation")
-    nam.add_option('--sitemap', type=str, action='store',
+    nam.add_argument('--sitemap', type=str, action='store',
-                   help="explicitly set sitemap name, overriding default sitemap.xml "
+                     help="explicitly set sitemap name, overriding default sitemap.xml "
-                        "appended to first source URI specified in the mappings")
+                          "appended to first source URI specified in the mappings")
-    nam.add_option('--reference', type=str, action='store',
+    nam.add_argument('--reference', type=str, action='store',
-                   help="reference sitemap name for --changelist calculation")
+                     help="reference sitemap name for --changelist calculation")
-    nam.add_option('--newreference', type=str, action='store',
+    nam.add_argument('--newreference', type=str, action='store',
-                   help="updated reference sitemap name for --changelist calculation")
+                     help="updated reference sitemap name for --changelist calculation")
-    nam.add_option('--dump', metavar='DUMPFILE', type=str, action='store',
+    nam.add_argument('--dump', metavar='DUMPFILE', type=str, action='store',
-                   help="write dump to specified file for --resourcelist or --changelist")
+                     help="write dump to specified file for --resourcelist or --changelist")
-    nam.add_option('--changelist-uri', '--change-list-uri', type=str, action='store',
+    nam.add_argument('--changelist-uri', '--change-list-uri', type=str, action='store',
-                   help="explicitly set the changelist URI that will be use in --inc mode, "
+                     help="explicitly set the changelist URI that will be use in --inc mode, "
-                        "overrides process of getting this from the sitemap")
+                          "overrides process of getting this from the sitemap")
 
     # Options that apply to multiple modes
-    opt = p.add_option_group('MISCELANEOUS OPTIONS')
+    opt = parser.add_argument_group('MISCELANEOUS OPTIONS')
-    opt.add_option('--max-sitemap-entries', type=int, action='store',
+    opt.add_argument('--max-sitemap-entries', type=int, action='store',
-                   help="override default size limits")
+                     help="override default size limits")
     add_shared_misc_options(opt, default_logfile=DEFAULT_LOGFILE)
 
-    (args, map) = p.parse_args()
+    args = parser.parse_args()
 
     init_logging(to_file=args.logger, logfile=args.logfile, default_logfile=DEFAULT_LOGFILE,
                  verbose=args.verbose)
@@ -76,9 +76,6 @@ def main():
                  verbose=args.verbose)
 
     try:
-        if (map):
-            # Mappings apply to (almost) everything
-            c.set_mappings(map)
         if (args.sitemap):
             c.sitemap_name = args.sitemap
         if (args.exclude):

resync-sync

@@ -0,0 +1,162 @@
+#!/usr/bin/env python
+"""resync-sync: The ResourceSync command line synchronization client.
+
+Copyright 2012-2020 Simeon Warner
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License
+"""
+
+import argparse
+import sys
+
+from resync import __version__
+from resync.client import Client, ClientFatalError
+from resync.client_utils import init_logging, count_true_args, parse_links, parse_capabilities, parse_capability_lists, add_shared_misc_options, process_shared_misc_options
+
+DEFAULT_LOGFILE = 'resync-client.log'
+
+
+def main():
+    """Main function to implement command line script."""
+    if (sys.version_info < (3, 5)):
+        sys.exit("This program requires python version 3.5 or later")
+
+    # Options and arguments
+    parser = argparse.ArgumentParser(
+        description="ResourceSync command line client (v" + __version__ + ")",
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter)
+
+    parser._optionals = parser.add_argument_group(
+        'MODES OF OPERATION (must specify one only). A source that is specified '
+        'either in a set of uri=path mappings or else using an explicit --sitemap '
+        'location')
+    rem = parser.add_mutually_exclusive_group(required=True)
+    rem.add_argument('--baseline', '-b', action='store_true',
+                     help='baseline sync of resources from remote source (src) to local filesystem (dst)')
+    rem.add_argument('--incremental', '--inc', '-i', action='store_true',
+                     help='incremental sync of resources from remote source (src) to local filesystem (dst). Uses either timestamp recorded from last baseline or incremental sync for this source, or explicit --from parameter, to determine the earlier update timestamp to act on.')
+    rem.add_argument('--audit', '-a', action='store_true',
+                     help="audit sync state of destination wrt source")
+    rem.add_argument('--parse', '-p', action='store_true',
+                     help="parse a remote sitemap/sitemapindex (from mapping or explicit --sitemap) and show summary information including document type and number of entries")
+
+    # Positional arguments
+    map = parser.add_argument_group('URI MAPPING TO FILESYSTEM for REMOTE modes')
+    map.add_argument(metavar='uri=path | uri path', dest='map', type=str, nargs='*',
+                     help="remote URI of source for remote synchronization operations (may "
+                          "also combine uri=local path)")
+
+    # Specification of map between remote URI and local file paths, and remote
+    # sitemap
+    nam = parser.add_argument_group('FILE/URI NAMING OPTIONS')
+    nam.add_argument('--sitemap', type=str, action='store',
+                     help="explicitly set sitemap name, overriding default sitemap.xml "
+                          "appended to first source URI specified in the mappings")
+    nam.add_argument('--capabilitylist', '--capability-list', type=str, action='store',
+                     help="explicitly set capability list URI to search for instead of "
+                          "looking for the source description")
+    nam.add_argument('--reference', type=str, action='store',
+                     help="reference sitemap name for --write-changelist calculation")
+    nam.add_argument('--newreference', type=str, action='store',
+                     help="updated reference sitemap name for --write-changelist calculation")
+    nam.add_argument('--changelist-uri', '--change-list-uri', type=str, action='store',
+                     help="explicitly set the changelist URI that will be use in --inc mode, "
+                          "overrides process of getting this from the sitemap")
+
+    # Options that apply to multiple modes
+    opt = parser.add_argument_group('MISCELANEOUS OPTIONS')
+    add_shared_misc_options(opt, default_logfile=DEFAULT_LOGFILE)
+    opt.add_argument('--delete', action='store_true',
+                     help="allow files on destination to be deleted")
+    opt.add_argument('--empty', action='store_true',
+                     help="combine with --changelist to write and empty changelist, perhaps with links")
+    opt.add_argument('--strictauth', action='store_true',
+                     help="use more strict checking of URLs to ensure that the ResourceSync "
+                          "documents refer only to resources on the same server or sub-domains, "
+                          "and on the same server to sub-paths. This is the authority model "
+                          "of Sitemaps but there are legitimate uses where these rules would "
+                          "not be followed.")
+    opt.add_argument('--dryrun', '-n', action='store_true',
+                     help="don't update local resources, say what would be done")
+    opt.add_argument('--ignore-failures', action='store_true',
+                     help="continue past download failures")
+    # These likely only useful for experimentation
+    opt.add_argument('--max-sitemap-entries', type=int, action='store',
+                     help="override default size limits")
+    opt.add_argument('--eval', '-e', action='store_true',
+                     help="output evaluation of source/client synchronization performance... "
+                          "be warned, this is very verbose")
+    opt.add_argument('--tries', '-t', type=int, action='store', metavar='TRIES',
+                     help="set number of tries to TRIES. The default is to retry 20 times, "
+                          "with the exception of fatal errors like \"connection refused\" "
+                          "or \"not found\" (404), which are not retried.")
+    opt.add_argument('--timeout', '-T', type=int, action='store', metavar='SECONDS',
+                     help="set the request timeout for resource downloads to SECONDS seconds")
+
+    args = parser.parse_args()
+
+    # Configure logging module and create logger instance
+    init_logging(to_file=args.logger, logfile=args.logfile, default_logfile=DEFAULT_LOGFILE,
+                 verbose=args.verbose, eval_mode=args.eval)
+
+    process_shared_misc_options(args)
+
+    c = Client(hashes=args.hash,
+               verbose=args.verbose,
+               dryrun=args.dryrun)
+
+    try:
+        if (args.map):
+            # Mappings apply to (almost) everything
+            c.set_mappings(args.map)
+        if (args.sitemap):
+            c.sitemap_name = args.sitemap
+        if (args.capabilitylist):
+            c.capability_list_uri = args.capabilitylist
+        if (args.exclude):
+            c.exclude_patterns = args.exclude
+        if (args.multifile):
+            c.allow_multifile = not args.multifile
+        if (args.noauth):
+            c.noauth = args.noauth
+        if (args.strictauth):
+            c.strictauth = args.strictauth
+        if (args.max_sitemap_entries):
+            c.max_sitemap_entries = args.max_sitemap_entries
+        if (args.ignore_failures):
+            c.ignore_failures = args.ignore_failures
+        if (args.tries):
+            c.tries = args.tries
+        if (args.timeout):
+            c.timeout = args.timeout
+
+        # Finally, do something...
+        if (args.baseline or args.audit):
+            c.baseline_or_audit(allow_deletion=args.delete,
+                                audit_only=args.audit)
+        elif (args.incremental):
+            c.incremental(allow_deletion=args.delete,
+                          change_list_uri=args.changelist_uri,
+                          from_datetime=args.from_datetime)
+        elif (args.parse):
+            c.parse_document()
+        else:
+            parser.error("Unknown mode requested")
+    # Any problem we expect will come as a ClientFatalError, anything else
+    # is... an exception ;-)
+    except ClientFatalError as e:
+        sys.stderr.write("\nFatalError: " + str(e) + "\n")
+
+
+if __name__ == '__main__':
+    main()

tests/test_resync-build_script.py

@@ -1,11 +1,6 @@
 import sys
 import unittest
-try:  # python2
+import io
-    # Must try this first as io also exists in python2
-    # but in the wrong one!
-    import BytesIO as io
-except ImportError:  # python3
-    import io
 
 from resync.resource import Resource
 from resync.resource_list import ResourceList
@@ -16,7 +11,7 @@ import subprocess
 
 
 def run_resync(args):
-    args.insert(0, 'bin/resync')
+    args.insert(0, './resync-build')
     proc = subprocess.Popen(args, stdout=subprocess.PIPE)
     (out, err) = proc.communicate()
     return(out)

tests/test_resync-explorer_script.py

@@ -10,7 +10,7 @@ except ImportError:  # python3
 
 
 def run_resync_explorer(args):
-    args.insert(0, 'bin/resync-explorer')
+    args.insert(0, './resync-explorer')
     proc = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     (out, err) = proc.communicate()
     return(out, err)
@@ -21,9 +21,9 @@ class TestClientLinkOptions(unittest.TestCase):
     def test01_help(self):
         """Check that it runs with -h."""
         txt = run_resync_explorer(['-h'])[0]
-        self.assertTrue(txt.startswith(b'Usage: resync-explorer [options] uri'))
+        self.assertRegex(txt, rb'''ResourceSync explorer \(''')
 
     def test02_error(self):
         """Bad parameter."""
         err = run_resync_explorer([])[1]
-        self.assertRegex(err, b'FatalError: No source information')
+        self.assertRegex(err, rb'''FatalError: No source information''')