From 7adecd8c4f7136fd23c536f9fe8f840a6311ada2 Mon Sep 17 00:00:00 2001 From: "Michael S. Noble" Date: Thu, 9 Aug 2018 11:23:00 -0400 Subject: [PATCH] sample_list now works for all sample container types, not just workspaces; added pair_list; 2 new HL tests; and capability to REUSE_SPACE between test invocations --- Makefile | 13 +++-- changelog.txt | 8 ++++ firecloud/__about__.py | 2 +- firecloud/fiss.py | 77 +++++++++++++++++++++++------- firecloud/tests/highlevel_tests.py | 56 +++++++++++++++++++++- setup.py | 1 + 6 files changed, 134 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index 10792a1..9486aa8 100644 --- a/Makefile +++ b/Makefile @@ -6,12 +6,9 @@ SHELL=/bin/bash __FILE__=$(lastword $(MAKEFILE_LIST)) __PATH__=$(abspath $(dir $(__FILE__))) ROOT=$(__PATH__) -PYTHON_HOME=$(shell $(ROOT)/util/findPython.sh) +PYTHON_HOME=$(shell $(ROOT)/util/findPython.sh python$(PYTHON_VER)) PYLINT=$(ROOT)/util/pylint_wrap.sh -ifeq ($(MAKELEVEL),0) -$(info Using Python from $(PYTHON_HOME)) -endif DEST=$(PYTHON_HOME) BIN_DIR=$(DEST)/bin # Python virtual environment here PYTHON=$(DEST)/bin/python$(PYTHON_VER) @@ -21,6 +18,8 @@ VERBOSITY_FISS=0 HIGHLEVEL_TESTS=--tests=firecloud/tests/highlevel_tests.py LOWLEVEL_TESTS=--tests=firecloud/tests/lowlevel_tests.py +$(info Now using Python from $(PYTHON)) + help: @echo @echo "Build/test/install FISS for FireCloud. Needs GNUmake 3.81 or later" @@ -54,8 +53,14 @@ WHICH= test_one: @# Example: make test_one WHICH=space_lock_unlock @# Example: make test_one WHICH=ping VERBOSITY_FISS=1 (shows API calls) + @# Example: make test_one WHICH=sample_list REUSE_SPACE=true (see below) @$(MAKE) invoke_tests TESTS=$(HIGHLEVEL_TESTS):TestFISSHighLevel.test_$(WHICH) +# By default the tests create, populate & eventually delete a custom workspace. +# To change this, set REUSE_SPACE to any value, either here or on CLI; then, +# after running tests the space will be kept AND subsequent invocations will +# run faster by not re-creating the space and/or re-loading data into it + invoke_tests: @FISS_TEST_VERBOSITY=$(VERBOSITY_FISS) \ $(PYTHON) setup.py nosetests --verbosity=$(VERBOSITY_NOSE) \ diff --git a/changelog.txt b/changelog.txt index e171f13..46274a7 100644 --- a/changelog.txt +++ b/changelog.txt @@ -3,6 +3,14 @@ Change Log for FISSFC: the (Fi)recloud (S)ervice (S)elector ======================================================================= Terms used below: HL = high level interface, LL = low level interface +v0.16.16 - Added pair_list HL command; enlarged scope of applicability of + sample_list, to not only workspaces but all sample container + types: pair, sample_set, workspace, participant; added pair_list + HL function and new tests for both [pair,sample]_list; internal + HL function __get_entities now paginates by default, to enable + more calls to complete robustly; HL tests can now REUSE_SPACE for + faster execution during repeated testing + v0.16.15 - Hotfix: Fixed bug preventing gcloud tool installation. v0.16.14 - Hotfix: Fixed warning messages that would cause errors if executed. diff --git a/firecloud/__about__.py b/firecloud/__about__.py index 3a66efc..bd6fd2c 100644 --- a/firecloud/__about__.py +++ b/firecloud/__about__.py @@ -1,2 +1,2 @@ # Package version -__version__ = "0.16.15" +__version__ = "0.16.16" diff --git a/firecloud/fiss.py b/firecloud/fiss.py index e0f4051..5fce93a 100644 --- a/firecloud/fiss.py +++ b/firecloud/fiss.py @@ -251,20 +251,51 @@ def entity_list(args): # # print(r.content) -def __get_entities(args, kind): - r = fapi.get_entities(args.project, args.workspace, kind) - fapi._check_response_code(r, 200) - return [ entity['name'] for entity in r.json() ] +def __get_entities(args, kind, page_size=1000): + entities = _entity_paginator(args.project, args.workspace, kind, + page_size=page_size) + return [ entity['name'] for entity in entities ] @fiss_cmd def participant_list(args): - """ List participants in a workspace. """ + ''' List participants defined within a workspace.''' return __get_entities(args, "participant") +@fiss_cmd +def pair_list(args): + ''' List pairs defined within a workspace.''' + return __get_entities(args, "pair") + @fiss_cmd def sample_list(args): - """ List samples in a workspace. """ - return __get_entities(args, "sample") + ''' List samples within a container. ''' + + # Case 1: retrieve samples within a named data entity + if args.entity_type and args.entity: + # Edge case: caller asked for samples within a sample + if args.entity_type == 'sample': + return args.entity + # Edge case: samples for a participant, which has to be done hard way + # by iteratiing over all samples (see firecloud/discussion/9648) + elif args.entity_type == 'participant': + samples = _entity_paginator(args.project, args.workspace, + 'sample', page_size=2000) + return [ e['name'] for e in samples if + e['attributes']['participant']['entityName'] == args.entity] + + # Otherwise retrieve the container entity + r = fapi.get_entity(args.project, args.workspace, args.entity_type, args.entity) + fapi._check_response_code(r, 200) + if args.entity_type == 'pair': + pair = r.json()['attributes'] + samples = [ pair['case_sample'], pair['control_sample'] ] + else: + samples = r.json()['attributes']["samples"]['items'] + + return [ sample['entityName'] for sample in samples ] + + # Case 2: retrieve all samples within a workspace + return __get_entities(args, "sample", page_size=2000) @fiss_cmd def sset_list(args): @@ -275,11 +306,10 @@ def sset_list(args): def entity_delete(args): """ Delete entity in a workspace. """ - prompt = "WARNING: this will delete {0} {1} in {2}/{3}".format( - args.entity_type, args.entity, args.project, args.workspace - ) + msg = "WARNING: this will delete {0} {1} in {2}/{3}".format( + args.entity_type, args.entity, args.project, args.workspace) - if not (args.yes or _confirm_prompt(prompt)): + if not (args.yes or _confirm_prompt(msg)): return json_body=[{"entityType": args.entity_type, @@ -909,7 +939,6 @@ def attr_fill_null(args): args.entity_type, attr ) if not args.yes and not _confirm_prompt(message, prompt): - #Don't do it! return # check to see if no sentinels are necessary @@ -1447,7 +1476,7 @@ def runnable(args): # Utilities ################################################# -def _confirm_prompt(message, prompt="\nAre you sure? [Y\\n]: ", +def _confirm_prompt(message, prompt="\nAre you sure? [y/yes (default: no)]: ", affirmations=("Y", "Yes", "yes", "y")): """ Display a message, then confirmation prompt, and return true @@ -1639,6 +1668,8 @@ def main(argv=None): meth_ns_required = not bool(fcconfig.method_ns) workspace_required = not bool(fcconfig.workspace) etype_required = not bool(fcconfig.entity_type) + etype_choices = ['participant', 'participant_set', 'sample', 'sample_set', + 'pair', 'pair_set' ] # Initialize core parser (TODO: Add longer description) usage = 'fissfc [OPTIONS] [CMD [-h | arg ...]]' @@ -1822,10 +1853,24 @@ def main(argv=None): parents=[workspace_parent]) subp.set_defaults(func=participant_list) - # List of samples + # List of pairs subp = subparsers.add_parser( - 'sample_list', description='List samples in a workspace', + 'pair_list', description='List pairs defined within a workspace', parents=[workspace_parent]) + subp.set_defaults(func=pair_list) + + # List of samples + subp = subparsers.add_parser( + 'sample_list', + parents=[workspace_parent], + description='Return the list of samples within a given container, ' \ + 'which by default is the workspace; otherwise, the samples within'\ + 'the named entity will be listed. If an entity type is not given '\ + 'then sample_set will be assumed.') + subp.add_argument('-e', '--entity', default=None, + help='Entity name, to list samples within container entities') + subp.add_argument('-t', '--entity-type', default='sample_set', + help='The type for named entity [default:%(default)s]`') subp.set_defaults(func=sample_list) # List of sample sets @@ -2011,8 +2056,6 @@ def main(argv=None): parents=[workspace_parent, attr_parent]) # etype_parent not used for attr_get, because entity type is optional - etype_choices = ['participant', 'participant_set', 'sample', 'sample_set', - 'pair', 'pair_set' ] subp.add_argument('-t', '--entity-type', choices=etype_choices, default='', required=False, help='Entity type to retrieve ' + 'annotations from.') diff --git a/firecloud/tests/highlevel_tests.py b/firecloud/tests/highlevel_tests.py index a3e34c6..ce1c166 100644 --- a/firecloud/tests/highlevel_tests.py +++ b/firecloud/tests/highlevel_tests.py @@ -59,6 +59,12 @@ def setUpClass(cls): # Set up a temp workspace for duration of tests. And in case a previous # test failed, we attempt to unlock & delete before creating anew cls.workspace = getuser() + '_FISS_TEST' + + ret = call_func("space_exists", "-p", cls.project, "-w", cls.workspace) + if ret == True and os.environ.get("REUSE_SPACE", None): + return + + print("\tCreating test workspace ...\n", file=sys.stderr) r = fapi.unlock_workspace(cls.project, cls.workspace) r = fapi.delete_workspace(cls.project, cls.workspace) r = fapi.create_workspace(cls.project, cls.workspace) @@ -67,7 +73,8 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): print("\nFinishing high-level CLI tests ...\n", file=sys.stderr) - r = fapi.delete_workspace(cls.project, cls.workspace) + if not os.environ.get("REUSE_SPACE", None): + fapi.delete_workspace(cls.project, cls.workspace) def test_health(self): # Should return a bytes of 'OK' @@ -174,6 +181,7 @@ def load_entities(self): if r.status_code != 404: raise RuntimeError("while determining if SS-NT sample_set exists") + print("\n\tLoading data entities for tests ...", file=sys.stderr) call_func("entity_import", "-p", self.project, "-w", self.workspace, "-f", os.path.join("firecloud", "tests", "participants.tsv")) call_func("entity_import", "-p", self.project, "-w", self.workspace, @@ -188,6 +196,8 @@ def load_entities(self): "-f", os.path.join("firecloud", "tests", "pairset_membership.tsv")) call_func("entity_import", "-p", self.project, "-w", self.workspace, "-f", os.path.join("firecloud", "tests", "pairset_attr.tsv")) + + print("\t... done loading data ...", file=sys.stderr) def test_entity_delete(self): self.load_entities() @@ -335,6 +345,50 @@ def test_config_ops(self): self.assertEqual(config['rootEntityType'], 'sample_set') self.assertEqual(config['methodConfigVersion'], 1) + def test_sample_list(self): + self.load_entities() + + # Sample set, by way of using default value for -t + result = call_func('sample_list', '-p', self.project, + '-w', self.workspace, '-e', 'SS-NT') + self.assertEqual(2000, len(result)) + self.assertIn('S-0-NT', result) + self.assertIn('S-501-NT', result) + self.assertIn('S-1999-NT',result) + + # Pair + result = call_func('sample_list', '-p', self.project, + '-w', self.workspace, '-e', 'PAIR-3', '-t', 'pair') + self.assertEqual(2, len(result)) + self.assertIn('S-3-TP', result) + self.assertIn('S-3-NT', result) + + # Participant + result = call_func('sample_list', '-p', self.project, + '-w', self.workspace, '-e', 'P-23', '-t', 'participant') + self.assertEqual(2, len(result)) + self.assertIn('S-23-NT', result) + self.assertIn('S-23-TP', result) + + # Workspace, by way of using all defaults (no entity or type args) + result = call_func('sample_list','-p',self.project,'-w',self.workspace) + self.assertEqual(4000, len(result)) + self.assertIn('S-0-TP', result) + self.assertIn('S-1000-TP', result) + self.assertIn('S-1999-TP', result) + self.assertIn('S-999-NT', result) + self.assertIn('S-1999-NT', result) + + def test_pair_list(self): + self.load_entities() + + result = call_func('pair_list', '-p', self.project, + '-w', self.workspace) + self.assertEqual(10, len(result)) + self.assertIn('PAIR-1', result) + self.assertIn('PAIR-5', result) + self.assertIn('PAIR-10', result) + def main(): nose.main() diff --git a/setup.py b/setup.py index f53c22a..e705c0b 100644 --- a/setup.py +++ b/setup.py @@ -139,6 +139,7 @@ def get_outputs(self): 'pydot', 'requests[security]', 'six', + 'nose', 'pylint==1.7.2' ], classifiers = [