Merge pull request #105 from broadinstitute/entity_listing

expand entity list capabilities, tests & performance/robustness for large calls

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) \

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.

firecloud/__about__.py

@@ -1,2 +1,2 @@
 # Package version
-__version__ = "0.16.15"
+__version__ = "0.16.16"

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.')

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()
 

setup.py

@@ -139,6 +139,7 @@ def get_outputs(self):
         'pydot',
         'requests[security]',
         'six',
+        'nose',
         'pylint==1.7.2'
     ],
     classifiers = [