From b90e58013a45a56f02c32453799ee46604df7deb Mon Sep 17 00:00:00 2001 From: jlchang Date: Mon, 31 Aug 2020 14:57:17 -0400 Subject: [PATCH 1/5] send user-agent string unless --no-version-check --- scripts/cli_parser.py | 230 ++++++++++++++++++++---------------- scripts/manage_study.py | 99 +++++++++------- scripts/scp_api.py | 253 ++++++++++++++++++++-------------------- 3 files changed, 312 insertions(+), 270 deletions(-) diff --git a/scripts/cli_parser.py b/scripts/cli_parser.py index cc808a7..8b580f8 100644 --- a/scripts/cli_parser.py +++ b/scripts/cli_parser.py @@ -13,202 +13,216 @@ c_TOOL_STUDY = "create-study" c_TOOL_STUDY_EDIT_DESC = "edit-study-description" c_TOOL_STUDY_GET_ATTR = "get-study-attribute" -c_TOOL_STUDY_GET_EXT = 'get-study-external-resources' -c_TOOL_STUDY_DEL_EXT = 'delete-study-external-resources' -c_TOOL_STUDY_CREATE_EXT = 'create-study-external-resource' +c_TOOL_STUDY_GET_EXT = "get-study-external-resources" +c_TOOL_STUDY_DEL_EXT = "delete-study-external-resources" +c_TOOL_STUDY_CREATE_EXT = "create-study-external-resource" def create_parser(): args = argparse.ArgumentParser( - prog='manage-study', + prog="manage-study", description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter, ) args.add_argument( - '--token', + "--token", default=None, - help='Personal token after logging into Google (OAuth2). This token is not persisted after the finish of the script.', + help="Personal token after logging into Google (OAuth2). This token is not persisted after the finish of the script.", ) args.add_argument( - '--dry-run', - action='store_true', - help='Walk through and log what would occur, without performing the actions.', + "--dry-run", + action="store_true", + help="Walk through and log what would occur, without performing the actions.", ) args.add_argument( - '--no-validate', - dest='validate', - action='store_false', - help='Do not check file locally before uploading.', + "--no-validate", + dest="validate", + action="store_false", + help="Do not check file locally before uploading.", ) args.add_argument( - '--verbose', action='store_true', help='Whether to print debugging information' + "--no-version-check", + dest="check_version", + action="store_false", + help="Do not check for ingest pipeline version.", + ) + args.add_argument( + "--verbose", action="store_true", help="Whether to print debugging information" ) args.add_argument( - '--environment', - default='production', - choices=['development', 'staging', 'production'], - help='API environment to use', + "--environment", + default="production", + choices=["development", "staging", "production"], + help="API environment to use", ) # Create tools (subparser) - subargs = args.add_subparsers(dest='command') + subargs = args.add_subparsers(dest="command") ## List studies subparser parser_list_studies = subargs.add_parser( c_TOOL_LIST_STUDY, - help="List studies. \"" + help='List studies. "' + args.prog + " " + c_TOOL_LIST_STUDY - + " -h\" for more details", + + ' -h" for more details', ) parser_list_studies.add_argument( - '--summary', - dest='summarize_list', - action='store_true', - help='Do not list, only summarize number of accessible studies', + "--summary", + dest="summarize_list", + action="store_true", + help="Do not list, only summarize number of accessible studies", ) ## Create study subparser parser_create_studies = subargs.add_parser( c_TOOL_STUDY, - help="Create a study. \"" + help='Create a study. "' + args.prog + " " + c_TOOL_STUDY - + " -h\" for more details", + + ' -h" for more details', ) parser_create_studies.add_argument( - '--description', - dest='study_description', + "--description", + dest="study_description", default="Single Cell Genomics Study", - help='Short description of the study', + help="Short description of the study", ) parser_create_studies.add_argument( - '--study-name', required=True, help='Short name of the study' + "--study-name", required=True, help="Short name of the study" ) parser_create_studies.add_argument( - '--branding', default=None, help='Portal branding to associate with the study', + "--branding", + default=None, + help="Portal branding to associate with the study", ) parser_create_studies.add_argument( - '--billing', + "--billing", default=None, - help='Portal billing project to associate with the study', + help="Portal billing project to associate with the study", ) parser_create_studies.add_argument( - '--is-private', action='store_true', help='Whether the study is private' + "--is-private", action="store_true", help="Whether the study is private" ) # Create edit description subparser parser_edit_description = subargs.add_parser( c_TOOL_STUDY_EDIT_DESC, - help="Edit a study description. \"" + help='Edit a study description. "' + args.prog + " " + c_TOOL_STUDY_EDIT_DESC - + " -h\" for more details", + + ' -h" for more details', ) parser_edit_description.add_argument( - '--study-name', + "--study-name", required=True, - help='Name of the study for which to edit description.', + help="Name of the study for which to edit description.", ) parser_edit_description.add_argument( - '--new-description', + "--new-description", required=True, - help='New description of the study to replace current one.', + help="New description of the study to replace current one.", ) parser_edit_description.add_argument( - '--from-file', - action='store_true', - help='If true, assumes new_description argument is name pointing to file containing new_description.', + "--from-file", + action="store_true", + help="If true, assumes new_description argument is name pointing to file containing new_description.", ) parser_edit_description.add_argument( - '--accept-html', - action='store_true', - help='If true, will allow HTML formatting in new description.', + "--accept-html", + action="store_true", + help="If true, will allow HTML formatting in new description.", ) ## Create study get attribute subparser parser_get_attribute = subargs.add_parser( c_TOOL_STUDY_GET_ATTR, - help="Get a study attribute (such as cell_count, etc). \"" + help='Get a study attribute (such as cell_count, etc). "' + args.prog + " " + c_TOOL_STUDY_GET_ATTR - + " -h\" for more details", + + ' -h" for more details', ) parser_get_attribute.add_argument( - '--study-name', + "--study-name", required=True, - help='Name of the study from which to get attribute.', + help="Name of the study from which to get attribute.", ) parser_get_attribute.add_argument( - '--attribute', + "--attribute", required=True, - help='Attribute to return (such as cell_count, etc).', + help="Attribute to return (such as cell_count, etc).", ) ## Create study get external resources subparser parser_get_ext_resources = subargs.add_parser( c_TOOL_STUDY_GET_EXT, - help="Get study external resources for a study. \"" + help='Get study external resources for a study. "' + args.prog + " " + c_TOOL_STUDY_GET_EXT - + " -h\" for more details", + + ' -h" for more details', ) parser_get_ext_resources.add_argument( - '--study-name', + "--study-name", required=True, - help='Name of the study from which to get resources.', + help="Name of the study from which to get resources.", ) ## Create study delete external resources subparser parser_delete_ext_resources = subargs.add_parser( c_TOOL_STUDY_DEL_EXT, - help="Delete all study external resources for a study. \"" + help='Delete all study external resources for a study. "' + args.prog + " " + c_TOOL_STUDY_DEL_EXT - + " -h\" for more details", + + ' -h" for more details', ) parser_delete_ext_resources.add_argument( - '--study-name', + "--study-name", required=True, - help='Name of the study from which to delete resources.', + help="Name of the study from which to delete resources.", ) ## Create study new external resource subparser parser_create_ext_resource = subargs.add_parser( c_TOOL_STUDY_CREATE_EXT, - help="Create a new external resource for a study. \"" + help='Create a new external resource for a study. "' + args.prog + " " + c_TOOL_STUDY_CREATE_EXT - + " -h\" for more details", + + ' -h" for more details', ) parser_create_ext_resource.add_argument( - '--study-name', + "--study-name", required=True, - help='Name of the study to which to add resource.', + help="Name of the study to which to add resource.", ) parser_create_ext_resource.add_argument( - '--title', required=True, help='Title of resource.', + "--title", + required=True, + help="Title of resource.", ) parser_create_ext_resource.add_argument( - '--url', required=True, help='URL of resource.', + "--url", + required=True, + help="URL of resource.", ) parser_create_ext_resource.add_argument( - '--description', required=True, help='Tooltip description of resource.', + "--description", + required=True, + help="Tooltip description of resource.", ) parser_create_ext_resource.add_argument( - '--publication-url', - action='store_true', - help='Whether resource is publication URL.', + "--publication-url", + action="store_true", + help="Whether resource is publication URL.", ) # TODO: Fix permissions subparser (SCP-2024) # ## Permissions subparser @@ -242,63 +256,71 @@ def create_parser(): ## Create cluster file upload subparser parser_upload_cluster = subargs.add_parser( c_TOOL_CLUSTER, - help="Upload a cluster file. \"" + help='Upload a cluster file. "' + args.prog + " " + c_TOOL_CLUSTER - + " -h\" for more details", + + ' -h" for more details', ) parser_upload_cluster.add_argument( - '--file', dest='cluster_file', required=True, help='Cluster file to load.' + "--file", dest="cluster_file", required=True, help="Cluster file to load." ) parser_upload_cluster.add_argument( - '--study-name', required=True, help='Name of the study to add the file.', + "--study-name", + required=True, + help="Name of the study to add the file.", ) parser_upload_cluster.add_argument( - '--description', + "--description", default="Coordinates and optional metadata to visualize clusters.", - help='Text describing the cluster file.', + help="Text describing the cluster file.", ) parser_upload_cluster.add_argument( - '--cluster-name', + "--cluster-name", required=True, - help='Name of the clustering that will be used to refer to the plot.', + help="Name of the clustering that will be used to refer to the plot.", ) parser_upload_cluster.add_argument( - '--x', dest='x_label', default=None, help='X axis label (test).' + "--x", dest="x_label", default=None, help="X axis label (test)." ) parser_upload_cluster.add_argument( - '--y', dest='y_label', default=None, help='Y axis label (test).' + "--y", dest="y_label", default=None, help="Y axis label (test)." ) parser_upload_cluster.add_argument( - '--z', dest='z_label', default=None, help='Z axis label (test).' + "--z", dest="z_label", default=None, help="Z axis label (test)." ) ## Create expression file upload subparser parser_upload_expression = subargs.add_parser( c_TOOL_EXPRESSION, - help="Upload a gene expression matrix file. \"" + help='Upload a gene expression matrix file. "' + args.prog + " " + c_TOOL_EXPRESSION - + " -h\" for more details", + + ' -h" for more details', ) parser_upload_expression.add_argument( - '--file', dest='expression_file', required=True, help='Expression file to load.' + "--file", dest="expression_file", required=True, help="Expression file to load." ) parser_upload_expression.add_argument( - '--study-name', required=True, help='Name of the study to add the file.', + "--study-name", + required=True, + help="Name of the study to add the file.", ) parser_upload_expression.add_argument( - '--description', - default='Gene expression in cells', - help='Text describing the gene expression matrix file.', + "--description", + default="Gene expression in cells", + help="Text describing the gene expression matrix file.", ) parser_upload_expression.add_argument( - '--species', required=True, help='Species from which the data is generated.', + "--species", + required=True, + help="Species from which the data is generated.", ) parser_upload_expression.add_argument( - '--genome', required=True, help='Genome assembly used to generate the data.', + "--genome", + required=True, + help="Genome assembly used to generate the data.", ) # TODO: Add upstream support for this in SCP RESI API # parser_upload_expression.add_argument( @@ -310,29 +332,33 @@ def create_parser(): ## Create metadata file upload subparser parser_upload_metadata = subargs.add_parser( c_TOOL_METADATA, - help="Upload a metadata file. \"" + help='Upload a metadata file. "' + args.prog + " " + c_TOOL_METADATA - + " -h\" for more details", + + ' -h" for more details', ) parser_upload_metadata.add_argument( - '--file', dest='metadata_file', required=True, help='Metadata file to load.' + "--file", dest="metadata_file", required=True, help="Metadata file to load." ) parser_upload_metadata.add_argument( - '--use-convention', - help='Whether to use metadata convention: validates against standard vocabularies, and will enable faceted search on this data', - action='store_true', + "--use-convention", + help="Whether to use metadata convention: validates against standard vocabularies, and will enable faceted search on this data", + action="store_true", ) parser_upload_metadata.add_argument( - '--validate-against-convention', - help='Validates against standard vocabularies prior to upload', - action='store_true', + "--validate-against-convention", + help="Validates against standard vocabularies prior to upload", + action="store_true", ) parser_upload_metadata.add_argument( - '--study-name', required=True, help='Name of the study to add the file.', + "--study-name", + required=True, + help="Name of the study to add the file.", ) parser_upload_metadata.add_argument( - '--description', default='', help='Text describing the metadata file.', + "--description", + default="", + help="Text describing the metadata file.", ) return args diff --git a/scripts/manage_study.py b/scripts/manage_study.py index 3aeee81..0c085bb 100644 --- a/scripts/manage_study.py +++ b/scripts/manage_study.py @@ -59,6 +59,7 @@ import json import os from bson.objectid import ObjectId +import pkg_resources from google.cloud import storage from ingest.ingest_pipeline import IngestPipeline @@ -82,23 +83,23 @@ from .cli_parser import * env_origin_map = { - 'development': 'https://localhost', - 'staging': 'https://singlecell-staging.broadinstitute.org', - 'production': 'https://singlecell.broadinstitute.org', + "development": "https://localhost", + "staging": "https://singlecell-staging.broadinstitute.org", + "production": "https://singlecell.broadinstitute.org", } def get_api_base(parsed_args): - return env_origin_map[parsed_args.environment] + '/single_cell/api/v1/' + return env_origin_map[parsed_args.environment] + "/single_cell/api/v1/" def manage_call_return(call_return, verbose=False): - ''' + """ Accesses the error codes in the underlying library and check the return code. :param call_return: Dict returned from scp_api call with REST call return info :return: No return will exit on error - ''' + """ # Print error code and describe code then exit if not success if verbose: print("HTTP status code = " + str(call_return[scp_api.c_CODE_RET_KEY])) @@ -112,23 +113,37 @@ def manage_call_return(call_return, verbose=False): def succeeded(ret): - """Whether request succeeded - """ + """Whether request succeeded""" # 2xx, e.g. 200 or 204, means success - return str(ret['response'].status_code)[0] == '2' + return str(ret["response"].status_code)[0] == "2" def login(parsed_args, manager=None, dry_run=False, api_base=None, verbose=False): - ''' + """ Login to authorize credentials. :param manager: API Manager :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: - ''' + """ if manager is None: manager = scp_api.SCPAPIManager(verbose=verbose) - manager.login(token=parsed_args.token, dry_run=dry_run, api_base=api_base) + if parsed_args.check_version: + ingest_pkg_version = pkg_resources.get_distribution( + "scp-ingest-pipeline" + ).version + portal_pkg_version = pkg_resources.get_distribution( + "single_cell_portal" + ).version + user_agent = f"single-cell-portal/{portal_pkg_version} (manage-study) scp-ingest-pipeline/{ingest_pkg_version} (ingest_pipeline.py)" + manager.login( + token=parsed_args.token, + dry_run=dry_run, + api_base=api_base, + user_agent=user_agent, + ) + else: + manager.login(token=parsed_args.token, dry_run=dry_run, api_base=api_base) return manager @@ -156,15 +171,15 @@ def validate_metadata_file(parsed_args, connection): dry_run = parsed_args.dry_run verbose = parsed_args.verbose study_accession_res = connection.get_study_attribute( - study_name=study_name, attribute='accession', dry_run=dry_run + study_name=study_name, attribute="accession", dry_run=dry_run ) # Needed dummy values for CellMetadata - study_file = ObjectId('addedfeed000000000000000') - study_file_id = ObjectId('addedfeed000000000000001') + study_file = ObjectId("addedfeed000000000000000") + study_file_id = ObjectId("addedfeed000000000000001") if succeeded(study_accession_res): if verbose: - print(f'Study accession {study_accession_res} retrieved for {study_name}') - study_accession = study_accession_res.get('study_attribute') + print(f"Study accession {study_accession_res} retrieved for {study_name}") + study_accession = study_accession_res.get("study_attribute") metadata = CellMetadata( metadata_path, study_file, @@ -173,12 +188,12 @@ def validate_metadata_file(parsed_args, connection): ) convention_res = connection.do_get( command=get_api_base(parsed_args) - + 'metadata_schemas/alexandria_convention/latest/json', + + "metadata_schemas/alexandria_convention/latest/json", dry_run=dry_run, ) if succeeded(convention_res): if verbose: - print(f'Retreieved file for latest metdata convention') + print(f"Retreieved file for latest metdata convention") convention = convention_res["response"].json() validate_input_metadata(metadata, convention) serialize_issues(metadata) @@ -188,9 +203,9 @@ def validate_metadata_file(parsed_args, connection): def confirm(question): while True: - answer = input(question + ' (y/n): ').lower().strip() - if answer in ('y', 'yes', 'n', 'no'): - return answer in ('y', 'yes') + answer = input(question + " (y/n): ").lower().strip() + if answer in ("y", "yes", "n", "no"): + return answer in ("y", "yes") def main(): @@ -203,7 +218,7 @@ def main(): verbose = parsed_args.verbose origin = env_origin_map[parsed_args.environment] - api_base = origin + '/single_cell/api/v1/' + api_base = origin + "/single_cell/api/v1/" # Login connection connection = login( @@ -243,7 +258,7 @@ def main(): ) manage_call_return(ret) if succeeded(ret): - print('Created study') + print("Created study") ## Get a study attribute if parsed_args.command == c_TOOL_STUDY_GET_ATTR: @@ -255,7 +270,7 @@ def main(): dry_run=parsed_args.dry_run, ) manage_call_return(ret) - print('Study {}:'.format(parsed_args.attribute)) + print("Study {}:".format(parsed_args.attribute)) print(ret[scp_api.c_ATTR_RET_KEY]) ## Edit a study description @@ -264,7 +279,7 @@ def main(): print("STARTING EDIT DESCRIPTION") if parsed_args.from_file: - with open(parsed_args.new_description, 'r') as d_file: + with open(parsed_args.new_description, "r") as d_file: new_description = d_file.read() else: new_description = parsed_args.new_description @@ -277,17 +292,18 @@ def main(): ) manage_call_return(ret) if succeeded(ret): - print('Updated study description') + print("Updated study description") ## Get all external resources from a study if parsed_args.command == c_TOOL_STUDY_GET_EXT: if verbose: print("STARTING GET EXTERNAL RESOURCES") ret = connection.get_study_external_resources( - study_name=parsed_args.study_name, dry_run=parsed_args.dry_run, + study_name=parsed_args.study_name, + dry_run=parsed_args.dry_run, ) manage_call_return(ret, verbose) - print('Study external resources: \n') + print("Study external resources: \n") # Returns in dict format -- table format might be better print(ret[scp_api.c_EXT_RET_KEY]) @@ -298,12 +314,13 @@ def main(): # first get all external resource ids ret = connection.get_study_external_resources( - study_name=parsed_args.study_name, dry_run=parsed_args.dry_run, + study_name=parsed_args.study_name, + dry_run=parsed_args.dry_run, ) manage_call_return(ret, verbose) - ext_ids = [res['_id']['$oid'] for res in ret['external_resources']] + ext_ids = [res["_id"]["$oid"] for res in ret["external_resources"]] if not ext_ids: - print('No external resources associated with study.') + print("No external resources associated with study.") exit() # confirm deletion with user confirmed = confirm( @@ -312,7 +329,7 @@ def main(): ) if confirmed: if verbose: - print('Will continue deleting resources.') + print("Will continue deleting resources.") for ext_id in ext_ids: ret = connection.delete_study_external_resource( study_name=parsed_args.study_name, @@ -320,7 +337,7 @@ def main(): dry_run=parsed_args.dry_run, ) manage_call_return(ret, verbose) - print('Deleted all external resources') + print("Deleted all external resources") ## Create new external resource for a study if parsed_args.command == c_TOOL_STUDY_CREATE_EXT: @@ -336,7 +353,7 @@ def main(): ) manage_call_return(ret, verbose) if succeeded(ret): - print('Created external resource') + print("Created external resource") ## Share with user if hasattr(parsed_args, "permission"): @@ -350,7 +367,7 @@ def main(): ) manage_call_return(ret) if succeeded(ret): - print('Set permission') + print("Set permission") ## Validate files if parsed_args.validate and not hasattr(parsed_args, "summarize_list"): @@ -396,14 +413,14 @@ def main(): ) manage_call_return(ret) if succeeded(ret): - print('Uploaded and began parse of cluster file') + print("Uploaded and began parse of cluster file") ## Upload metadata file if hasattr(parsed_args, "metadata_file"): if verbose: print("START UPLOAD METADATA FILE") connection = login(parsed_args, manager=connection, dry_run=parsed_args.dry_run) - print(f'connection is {connection}') + print(f"connection is {connection}") ret = connection.upload_metadata( file=parsed_args.metadata_file, use_convention=parsed_args.use_convention, @@ -412,7 +429,7 @@ def main(): ) manage_call_return(ret) if succeeded(ret): - print('Uploaded and began parse of metadata file') + print("Uploaded and began parse of metadata file") ## Upload expression file if hasattr(parsed_args, "expression_file"): @@ -428,7 +445,7 @@ def main(): ) manage_call_return(ret) if succeeded(ret): - print('Uploaded and began parse of expression file') + print("Uploaded and began parse of expression file") ## Upload miscellaneous file ### TODO @@ -446,5 +463,5 @@ def main(): ### TODO -if __name__ == '__main__': +if __name__ == "__main__": main() diff --git a/scripts/scp_api.py b/scripts/scp_api.py index ab5001b..3aa4cfb 100644 --- a/scripts/scp_api.py +++ b/scripts/scp_api.py @@ -24,14 +24,15 @@ from . import Commandline # Constants -c_AUTH = 'Authorization' +c_AGENT = "User-Agent" +c_AUTH = "Authorization" c_BOUNDARY_LENGTH = 20 c_CODE_RET_KEY = "code" c_RESPONSE = "response" c_STUDIES_RET_KEY = "studies" c_SUCCESS_RET_KEY = "success" -c_ATTR_RET_KEY = 'study_attribute' -c_EXT_RET_KEY = 'external_resources' +c_ATTR_RET_KEY = "study_attribute" +c_EXT_RET_KEY = "external_resources" ## Standard status codes c_STUDY_EXISTS = 101 @@ -98,9 +99,9 @@ class APIManager: - ''' + """ Base class for REST API interaction. Handles common operations. - ''' + """ def __init__(self): return @@ -109,7 +110,8 @@ def login( self, token=None, dry_run=False, - api_base='https://singlecell.broadinstitute.org/single_cell/api/v1/', + api_base="https://singlecell.broadinstitute.org/single_cell/api/v1/", + user_agent=False, ): """ Authenticates as user and get's token to perform actions on the user's behalf. @@ -123,20 +125,26 @@ def login( ## TODO add in auth from a file. if self.verbose: print("LOGIN") + print(f"user_agent: {user_agent}") if token is None: token = self.do_browser_login(dry_run=dry_run) self.token = token self.api_base = api_base - self.verify_https = 'localhost' not in api_base and 'staging' not in api_base + self.verify_https = "localhost" not in api_base and "staging" not in api_base self.studies = None + self.head = {"Accept": "application/json"} + if hasattr(self, "token"): + self.head[c_AUTH] = "Bearer {}".format(self.token) + if user_agent: + self.head[c_AGENT] = user_agent def do_browser_login(self, dry_run=False): - ''' + """ Authenticate through the browser :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Authentication token - ''' + """ if self.verbose: print("BROWSER LOGIN") @@ -151,30 +159,28 @@ def do_browser_login(self, dry_run=False): return cmd_ret.decode("ASCII").strip(os.linesep) def do_get(self, command, dry_run=False): - ''' + """ Perform a GET. :param command: String GET command to send to the REST endpoint :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Dict with response and status code - ''' + """ ## TODO add timeout and exception handling (Timeout exeception) if self.verbose: print("DO GET") print(command) + print(self.head) if dry_run: return {c_SUCCESS_RET_KEY: True, c_CODE_RET_KEY: c_API_OK} - head = {'Accept': 'application/json'} - if hasattr(self, 'token'): - head[c_AUTH] = 'Bearer {}'.format(self.token) return self.check_api_return( - requests.get(command, headers=head, verify=self.verify_https) + requests.get(command, headers=self.head, verify=self.verify_https) ) def do_post(self, command, values, files=None, dry_run=False): - ''' + """ *** In development *** Perform POST. @@ -183,7 +189,7 @@ def do_post(self, command, values, files=None, dry_run=False): :param files: :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Dict with response and status code - ''' + """ ## TODO addtimeout and exception handling (Timeout exeception) if self.verbose: @@ -194,20 +200,19 @@ def do_post(self, command, values, files=None, dry_run=False): if dry_run: print("DRY_RUN:: Returning success.") return {c_SUCCESS_RET_KEY: True, c_CODE_RET_KEY: c_API_OK} - head = {c_AUTH: 'token {}'.format(self.token), 'Accept': 'application/json'} if self.verbose: - print(head) + print(self.head) if files is None: return self.check_api_return( requests.post( - command, headers=head, json=values, verify=self.verify_https + command, headers=self.head, json=values, verify=self.verify_https ) ) else: return self.check_api_return( requests.post( command, - headers=head, + headers=self.head, files=files, json=values, verify=self.verify_https, @@ -215,14 +220,14 @@ def do_post(self, command, values, files=None, dry_run=False): ) def do_patch(self, command, values, dry_run=False): - ''' + """ Perform PATCH :param command: String PATCH command to send to the REST endpoint :param values: Parameter values to send {name: value} :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Dict with response and status code/status - ''' + """ ## TODO add timeout and exception handling (Timeout exeception) if self.verbose: @@ -231,19 +236,20 @@ def do_patch(self, command, values, dry_run=False): print(values) if dry_run: return {c_SUCCESS_RET_KEY: True, c_CODE_RET_KEY: c_API_OK} - head = {c_AUTH: 'token {}'.format(self.token), 'Accept': 'application/json'} return self.check_api_return( - requests.patch(command, headers=head, json=values, verify=self.verify_https) + requests.patch( + command, headers=self.head, json=values, verify=self.verify_https + ) ) def do_delete(self, command, dry_run=False): - ''' + """ Perform Delete :param command: String DELETE command to send to the REST endpoint :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Dict with response and status code/status - ''' + """ ## TODO add timeout and exception handling (Timeout exeception) if self.verbose: @@ -251,45 +257,44 @@ def do_delete(self, command, dry_run=False): print(command) if dry_run: return {c_SUCCESS_RET_KEY: True, c_CODE_RET_KEY: c_DELETE_OK} - head = {c_AUTH: 'token {}'.format(self.token), 'Accept': 'application/json'} return self.check_api_return( - requests.delete(command, headers=head, verify=self.verify_https) + requests.delete(command, headers=self.head, verify=self.verify_https) ) def check_api_return(self, ret): - ''' + """ Create dict that has if status code was successful, status code,and response :param ret: Response :return: Dict of response, status code, and status. - ''' + """ if self.verbose: - print(f'HTTP response status {ret.status_code}: {ret.reason}') + print(f"HTTP response status {ret.status_code}: {ret.reason}") api_return = {} api_return[c_SUCCESS_RET_KEY] = ret.status_code in [c_API_OK, c_DELETE_OK] if ret.status_code not in [c_API_OK, c_DELETE_OK]: - print(f'Error {ret.status_code}: {ret.reason}') + print(f"Error {ret.status_code}: {ret.reason}") if ret.status_code == 401: - print('') + print("") print( - 'Are you still signed in? SCP automatically logs you out after a period of inactivity.' + "Are you still signed in? SCP automatically logs you out after a period of inactivity." ) - print('Try logging in and re-assigning your access token like so:') - print('') - print('\tgcloud auth login') - print('\tACCESS_TOKEN=`gcloud auth print-access-token`') - print('') + print("Try logging in and re-assigning your access token like so:") + print("") + print("\tgcloud auth login") + print("\tACCESS_TOKEN=`gcloud auth print-access-token`") + print("") api_return[c_CODE_RET_KEY] = ret.status_code api_return[c_RESPONSE] = ret return api_return def get_boundary(self): - ''' + """ Creates a random string that is likely to not be in a MIME message. Used as a boundary between MIME parts. :return: String - ''' + """ base = string.ascii_lowercase + string.digits return "".join([random.choice(base) for i in range(c_BOUNDARY_LENGTH)]) @@ -297,14 +302,14 @@ def get_boundary(self): # The expected header class SCPAPIManager(APIManager): - ''' + """ API manager for the Single Cell Portal REST API - ''' + """ def __init__(self, verbose=False): - ''' + """ Initialize for the SCP endpoint. - ''' + """ APIManager.__init__(self) self.verbose = verbose @@ -330,12 +335,12 @@ def __init__(self, verbose=False): @staticmethod def describe_status_code(status_code): - ''' + """ Translate the status code to the text message, per SCP REST API documentation. :param status_code: Numeric status code to translate :return: String status code text - ''' + """ ret_status_codes = { c_STUDY_EXISTS: c_STUDY_EXISTS_TEXT, @@ -375,12 +380,12 @@ def check_species_genome(self, species, genome=None): ''' def get_studies(self, dry_run=False): - ''' + """ Get studies available to user. :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Response - ''' + """ if self.verbose: print("GET STUDIES") @@ -392,9 +397,9 @@ def get_studies(self, dry_run=False): if resp[c_SUCCESS_RET_KEY]: studies_json = resp[c_RESPONSE].json() self.study_objects = studies_json - self.studies = [str(element['name']) for element in studies_json] + self.studies = [str(element["name"]) for element in studies_json] self.name_to_id = [ - [str(element['name']), str(element['_id']['$oid'])] + [str(element["name"]), str(element["_id"]["$oid"])] for element in studies_json ] self.name_to_id = {key: value for (key, value) in self.name_to_id} @@ -402,12 +407,12 @@ def get_studies(self, dry_run=False): return resp def is_valid_study_description(self, study_description): - ''' + """ Confirms a study description does not contain characters that are not allowed. :param study_description: String description :return: Boolean indicator of validity, True is valid - ''' + """ no_error = True for letter in study_description: @@ -421,12 +426,12 @@ def is_valid_study_description(self, study_description): return no_error def is_valid_study_name(self, study_name): - ''' + """ Confirms a study name does not contain characters that are not allowed. :param study_name: String study name :return: Boolean indicator oc validity, True is valid - ''' + """ no_error = True for letter in study_name: @@ -440,13 +445,13 @@ def is_valid_study_name(self, study_name): def create_study( self, study_name, - study_description='Single Cell Genomics Study', + study_description="Single Cell Genomics Study", branding=None, billing=None, is_public=True, dry_run=False, ): - ''' + """ Create a study name using the REST API. Confirms the study does not exist before creating. Confirms name and description are valid. @@ -458,7 +463,7 @@ def create_study( :param is_public: Boolean indicator if the study should be public :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Response - ''' + """ if self.verbose: print("CREATE STUDY:: " + study_name) # Study variable validation @@ -476,10 +481,8 @@ def create_study( study_data = { "study": { "name": study_name, - "study_detail_attributes": { - "full_description": study_description - }, - "public": is_public + "study_detail_attributes": {"full_description": study_description}, + "public": is_public, } } if not branding is None: @@ -495,14 +498,14 @@ def create_study( return resp def get_study_attribute(self, study_name, attribute, dry_run=False): - ''' + """ Get a study attribute using the REST API. :param study_name: String study name :param attribute: String name of attribute to return :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Response - ''' + """ if self.verbose: print("LOOKING AT " + study_name) # Error if the study does not exist @@ -524,7 +527,7 @@ def get_study_attribute(self, study_name, attribute, dry_run=False): study = ret_study[c_RESPONSE].json() # Error if attribute is not available if attribute not in study: - print('The requested study attribute is not available.') + print("The requested study attribute is not available.") return { c_SUCCESS_RET_KEY: False, c_CODE_RET_KEY: c_ATTRIBUTE_DOES_NOT_EXIST, @@ -536,7 +539,7 @@ def get_study_attribute(self, study_name, attribute, dry_run=False): def edit_study_description( self, study_name, new_description, accept_html=False, dry_run=False ): - ''' + """ Edit a study description using the REST API. Confirms the study exists. @@ -545,7 +548,7 @@ def edit_study_description( :param accept_html: Boolean Whether to allow HTML characters and formatting :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Response - ''' + """ if self.verbose: print("EDITING DESCRIPTION FOR " + study_name) # Error if the study does not exist @@ -565,9 +568,7 @@ def edit_study_description( description_info = { "study": { "study_id": study_id, - "study_detail_attributes": { - "full_description": new_description - } + "study_detail_attributes": {"full_description": new_description}, } } @@ -580,13 +581,13 @@ def edit_study_description( return update_ret def get_study_external_resources(self, study_name, dry_run=False): - ''' + """ Get a study's external resources using the REST API. :param study_name: String study name :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Response - ''' + """ if self.verbose: print("LOOKING AT " + study_name) # Error if the study does not exist @@ -605,8 +606,8 @@ def get_study_external_resources(self, study_name, dry_run=False): if dry_run: print("DRY_RUN:: Returned dummy external resources.") study_resources[c_EXT_RET_KEY] = [ - {'_id': 'EXT_REC_DUMMY_1'}, - {'_id': 'EXT_REC_DUMMY_2'}, + {"_id": "EXT_REC_DUMMY_1"}, + {"_id": "EXT_REC_DUMMY_2"}, ] else: resources = study_resources[c_RESPONSE].json() @@ -615,14 +616,14 @@ def get_study_external_resources(self, study_name, dry_run=False): return study_resources def delete_study_external_resource(self, study_name, resource_id, dry_run=False): - ''' + """ Delete a study's external resource using the REST API. :param study_name: String study name :param resource_ids: String Id of external resource to delete :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Response - ''' + """ if self.verbose: print("LOOKING AT " + study_name) # Error if the study does not exist @@ -647,7 +648,7 @@ def delete_study_external_resource(self, study_name, resource_id, dry_run=False) def create_study_external_resource( self, study_name, title, url, description, publication_url=False, dry_run=False ): - ''' + """ Create an external resource for a study using the REST API. :param study_name: String study name @@ -657,7 +658,7 @@ def create_study_external_resource( :param publication_url Boolean Whether resource is publication :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Response - ''' + """ if self.verbose: print("CREATING RESOURCE FOR " + study_name) # Error if the study does not exist @@ -690,7 +691,7 @@ def create_study_external_resource( def set_permission( self, study_name, email, access, deliver_email=False, dry_run=False ): - ''' + """ Sets permission on a study. :param study_name: String study name to update permissions @@ -699,7 +700,7 @@ def set_permission( :param deliver_email: Boolean, is the user interested in email notifications for study changes (True, receive emails) :param dry_run: If true, will do a dry run with no actual execution of functionality :return: Dict with response and additional information including status and errors - ''' + """ if self.verbose: print("SET PERMISSION: ".join(str(i) for i in [study_name, email, access])) # Make sure the access value is valid @@ -722,7 +723,7 @@ def set_permission( share_id = None for share in ret_shares[c_RESPONSE].json(): if share["email"] == email: - share_id = share['_id']['$oid'] + share_id = share["_id"]["$oid"] permissions_info = { "study_id": studyId, @@ -771,13 +772,13 @@ def set_permission( return update_ret def study_exists(self, study_name, dry_run=False): - ''' + """ Indicates if the user has access to a study of the given name :param study_name: String study name :param dry_run: If true, will do a dry run with no actual execution of functionality :return: Boolean, True indicates the study is known to the user and exists - ''' + """ if self.verbose: print("STUDY EXISTS?") if self.studies is None: @@ -790,13 +791,13 @@ def study_exists(self, study_name, dry_run=False): return study_name in self.studies def study_name_to_id(self, name, dry_run=False): - ''' + """ Changes the a study name into the correct a portal id :param name: String study name :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: String portal id - ''' + """ if self.verbose: print("STUDY NAME TO ID") if dry_run: @@ -807,25 +808,24 @@ def study_name_to_id(self, name, dry_run=False): @staticmethod def upload_via_gsutil(bucket_id, file_path): - """Copy file to Google Cloud Storage bucket, return stats and filename - """ + """Copy file to Google Cloud Storage bucket, return stats and filename""" - gs_url = 'gs://' + bucket_id + gs_url = "gs://" + bucket_id # Upload to bucket via gsutil - command = 'gsutil cp ' + file_path + ' ' + gs_url + command = "gsutil cp " + file_path + " " + gs_url cmdline.func_CMD(command=command) - filename = file_path.split('/')[-1] + filename = file_path.split("/")[-1] # Get GCS details for the file just uploaded - command = 'gsutil stat ' + gs_url + '/' + filename + command = "gsutil stat " + gs_url + "/" + filename stdout = cmdline.func_CMD(command=command, stdout=True) # Split on newline, omit first and last lines, transform to dict - lines = [line.strip() for line in str(stdout).split('\\n')][1:-1] + lines = [line.strip() for line in str(stdout).split("\\n")][1:-1] gsutil_stat = {} for line in lines: - [key, value] = line.split(': ') + [key, value] = line.split(": ") gsutil_stat[key] = value.strip() return [gsutil_stat, filename] @@ -858,41 +858,41 @@ def upload_study_file( study_id = self.study_name_to_id(study_name, dry_run=dry_run) for s in self.study_objects: - if s['_id']['$oid'] == study_id: + if s["_id"]["$oid"] == study_id: study = s break - bucket_id = study['bucket_id'] + bucket_id = study["bucket_id"] [gsutil_stat, filename] = self.upload_via_gsutil(bucket_id, file) file_fields = { - 'study_file': { - 'file_type': file_type, - 'name': name, - 'remote_location': filename, - 'upload_file_name': filename, - 'upload_content_type': gsutil_stat['Content-Type'], - 'upload_file_size': int(gsutil_stat['Content-Length']), - 'generation': gsutil_stat['Generation'], + "study_file": { + "file_type": file_type, + "name": name, + "remote_location": filename, + "upload_file_name": filename, + "upload_content_type": gsutil_stat["Content-Type"], + "upload_file_size": int(gsutil_stat["Content-Length"]), + "generation": gsutil_stat["Generation"], } } for kwarg in kwargs: # Handles file-type specific fields, e.g. "species" and "genome" # for expression matrix files - file_fields['study_file'][kwarg] = kwargs[kwarg] + file_fields["study_file"][kwarg] = kwargs[kwarg] ret = self.do_post( - command=self.api_base + 'studies/' + study_id + '/study_files', + command=self.api_base + "studies/" + study_id + "/study_files", values=file_fields, dry_run=dry_run, ) # print(f'/study_files response: {ret["response"].text}') if parse: - study_files_response = ret['response'].json() - study_file_id = study_files_response['_id']['$oid'] + study_files_response = ret["response"].json() + study_file_id = study_files_response["_id"]["$oid"] parse = ( - f'{self.api_base}studies/{study_id}/study_files/{study_file_id}/parse' + f"{self.api_base}studies/{study_id}/study_files/{study_file_id}/parse" ) ret = self.do_post(command=parse, values=None, dry_run=dry_run) # print(f'/parse response: {ret["response"].text}') @@ -906,7 +906,7 @@ def upload_metadata( print("UPLOAD METADATA FILE") return self.upload_study_file( file, - 'Metadata', + "Metadata", study_name, description=description, parse=True, @@ -921,7 +921,7 @@ def upload_expression_matrix( print("UPLOAD EXPRESSION MATRIX FILE") return self.upload_study_file( file, - 'Expression Matrix', + "Expression Matrix", study_name, description=description, species=species, @@ -945,7 +945,7 @@ def upload_cluster( print("UPLOAD CLUSTER FILE") return self.upload_study_file( file, - 'Cluster', + "Cluster", study_name, cluster_name=cluster_name, description=description, @@ -958,14 +958,14 @@ def upload_cluster( class DSSAPIManager(APIManager): - ''' + """ API manager for the HCA DSS - ''' + """ def __init__(self): - ''' + """ Initialize for the DSS endpoint - ''' + """ APIManager.__init__(self) if self.verbose: @@ -974,14 +974,14 @@ def __init__(self): class MatrixAPIManager(APIManager): - ''' + """ API manager for Matrix service - ''' + """ def __init__(self): - ''' + """ Initialize for the expression matrix service - ''' + """ APIManager.__init__(self) print("INIT MATRIX API") @@ -989,12 +989,12 @@ def __init__(self): self.supportedTypes = None def describe_status_code(status_code): - ''' + """ Translate the status code to the text errors as per their endpoint documentation. :param status_code: Numeric status code to translate :return: String status code text - ''' + """ ret_status_codes = { c_MATRIX_API_OK: c_NO_ERROR_TEXT, @@ -1005,12 +1005,12 @@ def describe_status_code(status_code): return ret_status_codes.get(status_code, "That status code is not in use.") def get_supported_types(self, dry_run=False): - ''' + """ Query and update supported file types for delivery by the matrix service :param dry_run: If true, will do a dry run with no actual execution of functionality :return: Returns the response, also updates the supported types (in the Matrix Manager) - ''' + """ print("GET SUPPORTED TYPES") if dry_run: self.supportedTypes = ["test_type", "test_type"] @@ -1022,7 +1022,7 @@ def get_supported_types(self, dry_run=False): return resp def request_matrix(self, ids, format="zarr", dry_run=False): - ''' + """ *** In development **** Request a matrix by supplying bundle IDs @@ -1030,11 +1030,10 @@ def request_matrix(self, ids, format="zarr", dry_run=False): :param format: String supported file format to format the matrix to be received :param dry_run: If true, will do a dry run with no actual execution of functionality. :return: Response - ''' + """ print("REQUEST MATRIX BY IDs") if not format in self.get_supported_types(): return {c_SUCCESS_RET_KEY: False, c_CODE_RET_KEY: c_MATRIX_BAD_FORMAT} bundleInfo = {"bundle_fqids": ids, "format": format} resp = self.do_post(self.api, values=bundleInfo, dry_run=dry_run) return resp - From 8b506ce97e5bd936ab22c067aeee4f2610366e3d Mon Sep 17 00:00:00 2001 From: jlchang Date: Tue, 1 Sep 2020 16:06:54 -0400 Subject: [PATCH 2/5] --no-user-agent instead of --no-version-check prelim status 400 version mismatch error handling --- scripts/cli_parser.py | 6 +-- scripts/manage_study.py | 10 +++-- scripts/scp_api.py | 7 ++++ scripts/tests/test_scp_api.py | 75 ++++++++++++++++++++--------------- 4 files changed, 61 insertions(+), 37 deletions(-) diff --git a/scripts/cli_parser.py b/scripts/cli_parser.py index 8b580f8..04cb24c 100644 --- a/scripts/cli_parser.py +++ b/scripts/cli_parser.py @@ -41,10 +41,10 @@ def create_parser(): help="Do not check file locally before uploading.", ) args.add_argument( - "--no-version-check", - dest="check_version", + "--no-user-agent", + dest="user_agent", action="store_false", - help="Do not check for ingest pipeline version.", + help="Do not send user-agent string for ingest pipeline version.", ) args.add_argument( "--verbose", action="store_true", help="Whether to print debugging information" diff --git a/scripts/manage_study.py b/scripts/manage_study.py index 0c085bb..de8aa36 100644 --- a/scripts/manage_study.py +++ b/scripts/manage_study.py @@ -53,6 +53,10 @@ # Print a study attribute (eg. cell_count) python3 manage_study.py --token-$ACCESS_TOKEN get-study-attribute --study-name "${STUDY_NAME}" --attribute cell_count + +# Avoid sending a user-agent string while obtaining the number of studies in SCP +python manage_study.py --no-user-agent --token=$ACCESS_TOKEN list-studies --summary + """ import argparse @@ -128,7 +132,7 @@ def login(parsed_args, manager=None, dry_run=False, api_base=None, verbose=False """ if manager is None: manager = scp_api.SCPAPIManager(verbose=verbose) - if parsed_args.check_version: + if parsed_args.user_agent: ingest_pkg_version = pkg_resources.get_distribution( "scp-ingest-pipeline" ).version @@ -417,10 +421,10 @@ def main(): ## Upload metadata file if hasattr(parsed_args, "metadata_file"): + connection = login(parsed_args, manager=connection, dry_run=parsed_args.dry_run) if verbose: print("START UPLOAD METADATA FILE") - connection = login(parsed_args, manager=connection, dry_run=parsed_args.dry_run) - print(f"connection is {connection}") + print(f"connection is {connection}") ret = connection.upload_metadata( file=parsed_args.metadata_file, use_convention=parsed_args.use_convention, diff --git a/scripts/scp_api.py b/scripts/scp_api.py index 3aa4cfb..eaabfd3 100644 --- a/scripts/scp_api.py +++ b/scripts/scp_api.py @@ -285,6 +285,13 @@ def check_api_return(self, ret): print("\tgcloud auth login") print("\tACCESS_TOKEN=`gcloud auth print-access-token`") print("") + if ret.status_code == 400 and re.search( + "scp-ingest-pipeline.*incompatible", ret.reason + ): + print("") + print("Ingest pipeline package version incompatibility with server?") + print("") + exit(1) api_return[c_CODE_RET_KEY] = ret.status_code api_return[c_RESPONSE] = ret return api_return diff --git a/scripts/tests/test_scp_api.py b/scripts/tests/test_scp_api.py index 9934741..a6b6145 100644 --- a/scripts/tests/test_scp_api.py +++ b/scripts/tests/test_scp_api.py @@ -5,20 +5,22 @@ import re import sys -sys.path.append('.') + +sys.path.append(".") import scp_api # TODO: Incorporate into `test_upload_cluster` def mock_upload_via_gsutil(*args, **kwargs): gsutil_stat = { - 'Content-Type': 'test/foo', - 'Content-Length': '555', - 'Generation': 'foo' + "Content-Type": "test/foo", + "Content-Length": "555", + "Generation": "foo", } - filename = 'fake_path.txt' + filename = "fake_path.txt" return [gsutil_stat, filename] + # This method will be used by the mock to replace requests.get def mocked_requests_get(*args, **kwargs): class MockResponse: @@ -29,16 +31,17 @@ def __init__(self, json_data, status_code, reason=None): def json(self): return self.json_data - + url = args[0] - if url == 'https://singlecell.broadinstitute.org/single_cell/api/v1/studies': - with open('tests/data/studies.json') as f: + if url == "https://singlecell.broadinstitute.org/single_cell/api/v1/studies": + with open("tests/data/studies.json") as f: content = f.read() studies_json = json.loads(content, strict=False) - return MockResponse(studies_json, 200, reason='OK') + return MockResponse(studies_json, 200, reason="OK") return MockResponse(None, 404) + # This method will be used by the mock to replace requests.post def mocked_requests_post(*args, **kwargs): class MockResponse: @@ -52,50 +55,60 @@ def json(self): url = args[0] - study_files_url_re = '/v1/studies/.*/study_files$' + study_files_url_re = "/v1/studies/.*/study_files$" if re.search(study_files_url_re, url): - with open('tests/data/study_files_post.json') as f: + with open("tests/data/study_files_post.json") as f: content = f.read() study_files_json = json.loads(content, strict=False) - return MockResponse(study_files_json, 200, reason='OK') + return MockResponse(study_files_json, 200, reason="OK") - parse_url_re = '/v1/studies/.*/study_files/.*/parse$' + parse_url_re = "/v1/studies/.*/study_files/.*/parse$" if re.search(parse_url_re, url): return MockResponse(None, 204) return MockResponse(None, 404) -class SCPAPITestCase(unittest.TestCase): - @patch('requests.get', side_effect=mocked_requests_get) +class SCPAPITestCase(unittest.TestCase): + @patch("requests.get", side_effect=mocked_requests_get) def test_get_studies(self, mocked_requests_get): manager = scp_api.SCPAPIManager() - manager.api_base = 'https://singlecell.broadinstitute.org/single_cell/api/v1/' + manager.api_base = "https://singlecell.broadinstitute.org/single_cell/api/v1/" manager.verify_https = True - studies = manager.get_studies()['studies'] + manager.head = {"Accept": "application/json"} + studies = manager.get_studies()["studies"] expected_studies = [ " Single nucleus RNA-seq of cell diversity in the adult mouse hippocampus (sNuc-Seq)", - "Study only for unit test" + "Study only for unit test", ] self.assertEqual(studies, expected_studies) - @patch('scp_api.SCPAPIManager.upload_via_gsutil', side_effect=mock_upload_via_gsutil) - @patch('requests.post', side_effect=mocked_requests_post) - @patch('requests.get', side_effect=mocked_requests_get) - def test_upload_cluster(self, mocked_requests_get, mocked_requests_post, mock_upload_via_gsutil): + @patch( + "scp_api.SCPAPIManager.upload_via_gsutil", side_effect=mock_upload_via_gsutil + ) + @patch("requests.post", side_effect=mocked_requests_post) + @patch("requests.get", side_effect=mocked_requests_get) + def test_upload_cluster( + self, mocked_requests_get, mocked_requests_post, mock_upload_via_gsutil + ): # manager = scp_api.SCPAPIManager(verbose=True) manager = scp_api.SCPAPIManager() - manager.api_base = 'https://singlecell.broadinstitute.org/single_cell/api/v1/' + manager.api_base = "https://singlecell.broadinstitute.org/single_cell/api/v1/" manager.verify_https = True manager.login(dry_run=True) - return_object = manager.upload_cluster(file='../tests/data/toy_cluster', - study_name='Study only for unit test', - cluster_name='Test', - description='Test', - x='X', y='Y', z='Z') + return_object = manager.upload_cluster( + file="../tests/data/toy_cluster", + study_name="Study only for unit test", + cluster_name="Test", + description="Test", + x="X", + y="Y", + z="Z", + ) # HTTP 204 indicates successful parse launch, per # https://singlecell.broadinstitute.org/single_cell/api/swagger_docs/v1#!/StudyFiles/parse_study_study_file_path - self.assertEqual(return_object['code'], 204) + self.assertEqual(return_object["code"], 204) + -if __name__ == '__main__': - unittest.main() \ No newline at end of file +if __name__ == "__main__": + unittest.main() From 374e65a9c318247a5cc480a257656910b3d7b2a1 Mon Sep 17 00:00:00 2001 From: jlchang Date: Wed, 2 Sep 2020 13:03:33 -0400 Subject: [PATCH 3/5] rename test for clarity --- scripts/scp_api.py | 12 ++++++++---- scripts/tests/test_scp_api.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/scripts/scp_api.py b/scripts/scp_api.py index eaabfd3..b134681 100644 --- a/scripts/scp_api.py +++ b/scripts/scp_api.py @@ -13,6 +13,7 @@ import string import os import json +import re import logging @@ -285,11 +286,13 @@ def check_api_return(self, ret): print("\tgcloud auth login") print("\tACCESS_TOKEN=`gcloud auth print-access-token`") print("") - if ret.status_code == 400 and re.search( - "scp-ingest-pipeline.*incompatible", ret.reason - ): + incompatible_ingest_version = re.search( + "scp-ingest-pipeline.*incompatible", ret.json()["error"] + ) + if ret.status_code == 400 and incompatible_ingest_version: print("") - print("Ingest pipeline package version incompatibility with server?") + print("Local ingest pipeline package version incompatibile with server") + print(ret.json()["error"]) print("") exit(1) api_return[c_CODE_RET_KEY] = ret.status_code @@ -403,6 +406,7 @@ def get_studies(self, dry_run=False): else: if resp[c_SUCCESS_RET_KEY]: studies_json = resp[c_RESPONSE].json() + print(f"from scp_api {studies_json}") self.study_objects = studies_json self.studies = [str(element["name"]) for element in studies_json] self.name_to_id = [ diff --git a/scripts/tests/test_scp_api.py b/scripts/tests/test_scp_api.py index a6b6145..2acdd3d 100644 --- a/scripts/tests/test_scp_api.py +++ b/scripts/tests/test_scp_api.py @@ -69,6 +69,24 @@ def json(self): return MockResponse(None, 404) +# This method will be used by the mock to replace requests.get response +# for ingest pipeline major version mismatch response in user-agent string +def mocked_ingest_version_mismatch(*args, **kwargs): + class MockResponse: + def __init__(self, json_data, status_code, reason=None): + self.json_data = json_data + self.status_code = status_code + self.reason = reason + + def json(self): + return self.json_data + + ingest_mismatch_json = { + "error": 'scp-ingest-pipeline: 0.9.12 incompatible with host, please update via "pip install scp-ingest-pipeline --upgrade"' + } + return MockResponse(ingest_mismatch_json, 400) + + class SCPAPITestCase(unittest.TestCase): @patch("requests.get", side_effect=mocked_requests_get) def test_get_studies(self, mocked_requests_get): @@ -83,6 +101,23 @@ def test_get_studies(self, mocked_requests_get): ] self.assertEqual(studies, expected_studies) + @patch("requests.get", side_effect=mocked_ingest_version_mismatch) + def test_ingest_version_mismatch(self, mocked_ingest_version_mismatch): + manager = scp_api.SCPAPIManager() + manager.api_base = "https://singlecell.broadinstitute.org/single_cell/api/v1/" + manager.verify_https = True + manager.head = { + "Accept": "application/json", + "User-Agent": "single-cell-portal/0.1.3rc1 (manage-study) scp-ingest-pipeline/0.9.12 (ingest_pipeline.py)", + } + + with self.assertRaises(SystemExit) as cm: + manager.get_studies()["studies"] + + self.assertEqual( + cm.exception.code, 1 + ), "expect exit if ingest major version mismatch detected" + @patch( "scp_api.SCPAPIManager.upload_via_gsutil", side_effect=mock_upload_via_gsutil ) From 78bc2638fe29ec8a0928c637b49508cf2a133ece Mon Sep 17 00:00:00 2001 From: jlchang Date: Wed, 2 Sep 2020 13:53:33 -0400 Subject: [PATCH 4/5] move get_user_agent to function --- scripts/manage_study.py | 26 +++++++++++++++++++------- scripts/scp_api.py | 5 ++--- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/scripts/manage_study.py b/scripts/manage_study.py index de8aa36..dbcb104 100644 --- a/scripts/manage_study.py +++ b/scripts/manage_study.py @@ -133,13 +133,7 @@ def login(parsed_args, manager=None, dry_run=False, api_base=None, verbose=False if manager is None: manager = scp_api.SCPAPIManager(verbose=verbose) if parsed_args.user_agent: - ingest_pkg_version = pkg_resources.get_distribution( - "scp-ingest-pipeline" - ).version - portal_pkg_version = pkg_resources.get_distribution( - "single_cell_portal" - ).version - user_agent = f"single-cell-portal/{portal_pkg_version} (manage-study) scp-ingest-pipeline/{ingest_pkg_version} (ingest_pipeline.py)" + user_agent = get_user_agent() manager.login( token=parsed_args.token, dry_run=dry_run, @@ -151,6 +145,24 @@ def login(parsed_args, manager=None, dry_run=False, api_base=None, verbose=False return manager +def get_user_agent(): + """Generate User-Agent string to reflect locally installed package versions""" + try: + ingest_pkg_version = pkg_resources.get_distribution( + "scp-ingest-pipeline" + ).version + except pkg_resources.DistributionNotFound: + ingest_pkg_version = None + try: + portal_pkg_version = pkg_resources.get_distribution( + "single_cell_portal" + ).version + except pkg_resources.DistributionNotFound: + portal_pkg_version = None + user_agent = f"single-cell-portal/{portal_pkg_version} (manage-study) scp-ingest-pipeline/{ingest_pkg_version} (ingest_pipeline.py)" + return user_agent + + def download_from_bucket(file_path): """Downloads file from Google Cloud Storage bucket""" diff --git a/scripts/scp_api.py b/scripts/scp_api.py index b134681..03e96df 100644 --- a/scripts/scp_api.py +++ b/scripts/scp_api.py @@ -286,10 +286,9 @@ def check_api_return(self, ret): print("\tgcloud auth login") print("\tACCESS_TOKEN=`gcloud auth print-access-token`") print("") - incompatible_ingest_version = re.search( + if ret.status_code == 400 and re.search( "scp-ingest-pipeline.*incompatible", ret.json()["error"] - ) - if ret.status_code == 400 and incompatible_ingest_version: + ): print("") print("Local ingest pipeline package version incompatibile with server") print(ret.json()["error"]) From f1a3e1ec810890c32fd5ecb872df06d7829d883e Mon Sep 17 00:00:00 2001 From: jlchang Date: Wed, 2 Sep 2020 15:32:20 -0400 Subject: [PATCH 5/5] update setup.py for release to PyPI --- setup.py | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/setup.py b/setup.py index abda2d0..e201b41 100644 --- a/setup.py +++ b/setup.py @@ -1,25 +1,27 @@ from setuptools import setup, find_packages -with open('README.md', 'r') as fh: +with open("README.md", "r") as fh: long_description = fh.read() setup( - name='single-cell-portal', - version='0.1.2', - description='Convenience scripts for Single Cell Portal', + name="single-cell-portal", + version="0.2.0", + description="Convenience scripts for Single Cell Portal", long_description=long_description, - long_description_content_type='text/markdown', - url='https://github.com/broadinstitute/single_cell_portal', - author='Single Cell Portal team', - author_email='scp-support@broadinstitute.zendesk.com', - install_requires=['pandas', 'requests', 'scp-ingest-pipeline==1.5.4'], + long_description_content_type="text/markdown", + url="https://github.com/broadinstitute/single_cell_portal", + author="Single Cell Portal team", + author_email="scp-support@broadinstitute.zendesk.com", + install_requires=["pandas", "requests", "scp-ingest-pipeline==1.5.4"], packages=find_packages(), - entry_points={'console_scripts': ['manage-study=scripts.manage_study:main'],}, + entry_points={ + "console_scripts": ["manage-study=scripts.manage_study:main"], + }, classifiers=[ - 'Development Status :: 4 - Beta', - 'License :: OSI Approved :: BSD License', - 'Topic :: Scientific/Engineering :: Bio-Informatics', - 'Programming Language :: Python :: 3.7', + "Development Status :: 4 - Beta", + "License :: OSI Approved :: BSD License", + "Topic :: Scientific/Engineering :: Bio-Informatics", + "Programming Language :: Python :: 3.7", ], - python_requires='>=3.7', + python_requires=">=3.7", )