Improvements to package dependency lists (#463)

* Improvements to Python package dependencies
  - Add comments noting best practices for dependency management
  - Format dependency version ranges
  - Remove transitive (sub) dependencies
  - Update dependency minimum versions to those distributed by Ubuntu 18.04 where available
  - Bound version ranges in a consistent manner and leave comments where pins are necessary

* Fix lint issues and un-silence linter on several error types

* AWS CLI is a development dependency

hca/cli.py

@@ -35,7 +35,7 @@ def add_parser_func(self, func, **kwargs):
             self._subparsers = self.add_subparsers()
         subparser = self._subparsers.add_parser(func.__name__.replace("_", "-"), **kwargs)
         subparser.set_defaults(entry_point=func)
-        command = subparser.prog[len(self.prog)+1:].replace("-", "_").replace(" ", "_")
+        command = subparser.prog[len(self.prog) + 1:].replace("-", "_").replace(" ", "_")
         subparser.set_defaults(**get_config().get(command, {}))
         if subparser.description is None:
             subparser.description = kwargs.get("help", func.__doc__)

hca/dss/__init__.py

@@ -78,7 +78,8 @@ class DSSClient(SwaggerClient):
 
     def __init__(self, *args, **kwargs):
         super(DSSClient, self).__init__(*args, **kwargs)
-        self.commands += [self.upload, self.download, self.download_manifest, self.create_version, self.download_collection]
+        self.commands += [self.upload, self.download, self.download_manifest, self.create_version,
+                          self.download_collection]
 
     def create_version(self):
         """
@@ -115,7 +116,8 @@ def upload(self, src_dir, replica, staging_bucket, timeout_seconds=1200, no_prog
 
         logger.info("Uploading %i files from %s to %s", len(files_to_upload), src_dir, staging_bucket)
         file_uuids, uploaded_keys, abs_file_paths = upload_to_cloud(files_to_upload, staging_bucket=staging_bucket,
-                                                                    replica=replica, from_cloud=False, log_progress=not no_progress)
+                                                                    replica=replica, from_cloud=False,
+                                                                    log_progress=not no_progress)
         for file_handle in files_to_upload:
             file_handle.close()
         filenames = [object_name_builder(p, src_dir) for p in abs_file_paths]
@@ -636,7 +638,7 @@ def _do_download_file(self, dss_file, fh):
                     server_start = 0
                     content_range_header = response.headers.get('Content-Range', None)
                     if content_range_header is not None:
-                        cre = re.compile("bytes (\d+)-(\d+)")
+                        cre = re.compile(r"bytes (\d+)-(\d+)")
                         mo = cre.search(content_range_header)
                         if mo is not None:
                             server_start = int(mo.group(1))
@@ -651,13 +653,13 @@ def _do_download_file(self, dss_file, fh):
                             dss_file.uuid, server_start, consume_bytes))
 
                         while consume_bytes > 0:
-                            bytes_to_read = min(consume_bytes, 1024*1024)
+                            bytes_to_read = min(consume_bytes, 1024 * 1024)
                             content = response.iter_content(chunk_size=bytes_to_read)
                             chunk = next(content)
                             if chunk:
                                 consume_bytes -= len(chunk)
 
-                    for chunk in response.iter_content(chunk_size=1024*1024):
+                    for chunk in response.iter_content(chunk_size=1024 * 1024):
                         if chunk:
                             fh.write(chunk)
                             hasher.update(chunk)

hca/dss/cli.py

@@ -1,5 +1,6 @@
 from . import DSSClient
 
+
 def add_commands(subparsers, help_menu=False):
     dss_parser = subparsers.add_parser('dss', help="Interact with the HCA Data Storage System")
 

hca/query/cli.py

@@ -1,5 +1,6 @@
 from . import DCPQueryClient
 
+
 def add_commands(subparsers, help_menu=False):
     query_parser = subparsers.add_parser('query', help="Interact with the HCA DCP Query Service")
 

hca/upload/cli/upload_command.py

@@ -23,11 +23,11 @@ def add_parser(cls, upload_subparsers):
         upload_parser.add_argument('upload_paths', nargs='+', metavar="<upload_path>",
                                    help="Path to files or directories to be uploaded.")
         upload_parser.add_argument('-t', '--target-filename', metavar="<filename>", default=None,
-                                   help="Filename to use in upload area (if you wish to change it during upload)." +
-                                        " Only valid when one file is being uploaded.")
+                                   help=("Filename to use in upload area (if you wish to change it during upload)."
+                                         " Only valid when one file is being uploaded."))
         upload_parser.add_argument('--file-extension', metavar="<fileextension>", default=None,
-                                   help="File extension to limit which files should be uploaded" +
-                                        " Only valid when directories are targeted for upload.")
+                                   help=("File extension to limit which files should be uploaded"
+                                         " Only valid when directories are targeted for upload."))
         upload_parser.add_argument('--no-transfer-acceleration', action='store_true',
                                    help="""Don't use Amazon S3 Transfer Acceleration.
                                            By default we using the aforementioned service to upload via an endpoint
@@ -65,7 +65,7 @@ def __init__(self, args):
     def _load_config(self):
         self.config = UploadService.config()
         if not self.config.current_area:
-            sys.stderr.write("\nThere is no upload area selected.\n" +
+            sys.stderr.write("\nThere is no upload area selected.\n"
                              "Please select one with \"{cmdname} upload select <uri_or_alias>\"\n\n".format(
                                  cmdname=sys.argv[0]))
             exit(1)

hca/util/__init__.py

@@ -472,9 +472,10 @@ def _parse_properties(properties, schema):
                     anno = typing.Optional[anno]
                 param = Parameter(prop_name, Parameter.POSITIONAL_OR_KEYWORD, default=prop_data.get("default"),
                                   annotation=anno)
-                method_args.setdefault(prop_name, {}).update(dict(param=param, doc=prop_data.get("description"),
-                                                                  choices=enum_values,
-                                                                  required=prop_name in body_json_schema.get("required", [])))
+                method_args.setdefault(prop_name, {}).update(param=param,
+                                                             doc=prop_data.get("description"),
+                                                             choices=enum_values,
+                                                             required=prop_name in body_json_schema.get("required", []))
                 body_props[prop_name] = _merge_dict(schema, body_props.get('prop_name', {}))
 
         if body_json_schema.get('properties', {}):

hca/util/_docs.py

@@ -41,6 +41,7 @@
  ``{client_name}.{method_name}()`` listed here.
 """
 
+
 def _md2rst(docstring):
     parser = commonmark.Parser()
     ast = parser.parse(docstring)
@@ -89,9 +90,9 @@ def get_params(field_list_node, params):
 
     method_args = {'summary': '', 'params': dict(), 'description': ''}
     for node in document.children:
-        if node.tagname is 'paragraph' and method_args['summary'] == '':
+        if node.tagname == 'paragraph' and method_args['summary'] == '':
             method_args['summary'] = node.astext()
-        elif node.tagname is 'field_list':
+        elif node.tagname == 'field_list':
             get_params(node, method_args['params'])
         else:
             method_args['description'] += '\n' + node.astext()

hca/util/exceptions.py

@@ -1,5 +1,6 @@
 from requests.exceptions import HTTPError
 
+
 class SwaggerAPIException(HTTPError):
     """
     Exception raised by SwaggerClient when an HTTP error is received from a Swagger API server.
@@ -26,5 +27,6 @@ def __str__(self):
             return "{}: {} (HTTP {}). Details:\n{}".format(self.reason, self.title, self.code, self.response.text)
         return "{}, code {}".format(self.response.reason, self.response.status_code)
 
+
 class SwaggerClientInternalError(Exception):
     pass

requirements-dev.txt

@@ -1,7 +1,15 @@
+# List development (test or documentation) dependencies here.
+# Runtime dependencies should be listed in requirements.txt.
+# See the comment in requirements.txt on managing dependencies and their versions.
+
+awscli
 coverage
-flake8==3.5.0
+flake8
 mock
-moto>1.3.3
+
+# Old versions of moto do not mock boto3 objects with sufficient fidelity.
+moto > 1.3.3
+
 pyyaml
 responses
 Sphinx

requirements.txt

@@ -1,22 +1,37 @@
+# List runtime dependencies here.
+# Do not pin specific versions of dependencies unless absolutely necessary.
+# Instead, set bounds on major version ranges of dependencies in accordance with SemVer (https://semver.org).
+# If you must pin a specific version of a dependency, note the reason to do so in a comment line
+# immediately preceding the dependency.
+
+# Only dependencies which are directly imported by this library should be listed here. Dependencies
+# should manage their sub-dependencies. (If a sub-dependency must be restricted to a specific version
+# range, and upstream won't do so, note this in a comment.)
+
 argcomplete >= 1.9.3, < 2
-atomicwrites >=1.3.0, <2
-awscli>1.15.70
-boto3 > 1.8
-botocore>=1.12.13
+atomicwrites >= 1.3.0, < 2
+boto3 >= 1.9.86, < 2
+botocore >= 1.12.208, < 2
 commonmark >= 0.9.0, < 1
 cryptography >= 2.6.1, < 3
 dcplib >= 2.0.2, < 3
-docutils==0.14
-google-auth >= 1.0.2, < 2
-google-auth-oauthlib >= 0.1, < 2
-Jinja2 >= 2.9, < 3
+
+# The version range of docutils is pinned by botocore. Pinning it here can cause a version conflict.
+# See https://github.com/boto/botocore/pull/1802, https://github.com/HumanCellAtlas/dcp-cli/issues/418
+docutils
+
+google-auth >= 1.3.0, < 2
+google-auth-oauthlib >= 0.4.1, < 2
+Jinja2 >= 2.10, < 3
 jsonpointer >= 1.10, < 2
 jsonschema >= 2.6, < 3
-puremagic < 1.5
-PyJWT >= 1.6.4
+
+# Pinned due to an incompatibility with later versions of puremagic.
+# See https://github.com/HumanCellAtlas/dcp-cli/pull/245.
+puremagic==1.5
+
+PyJWT >= 1.6.4, < 2
 requests >= 2.20.0, < 3
-rsa<=3.5.0,>=3.1.2
-s3transfer<0.3.0,>=0.2.0
-tenacity >=5.0.2, < 5.1
-tqdm >=4.33.0, < 5
-tweak >= 1.0.2, < 2
+tenacity >= 5.0.2, < 5.1
+tqdm >= 4.33.0, < 5
+tweak >= 1.0.3, < 2

setup.cfg

@@ -2,4 +2,4 @@
 universal=1
 [flake8]
 max-line-length=120
-ignore: E301, E302, E401, E261, E265, E226, F401, E501
+ignore: F401, W504