Merge pull request #178 from cBioPortal/main-add-enable-study-cbioportalimporter

Keep studies UNAVAILABLE until derived tables are rebuilt

Author: alisman

pom.xml

@@ -17,7 +17,7 @@
 
 		<!-- THIS SHOULD BE KEPT IN SYNC WITH db.version IN cbioportal/pom.xml and
 		     info.db_schema_version IN cbioportal/src/main/resources/db-scripts/clickhouse/init/schema.sql -->
-		<db.version>3.0.0</db.version>
+		<db.version>3.0.1</db.version>
 		<cbioportal.version>v6.4.1</cbioportal.version>
 		<maf.version>1.0.0</maf.version>
 

scripts/importer/cbioportalImporter.py

@@ -58,8 +58,9 @@
 REMOVE_PATIENTS = "remove-patients"
 IMPORT_STUDY_DATA = "import-study-data"
 IMPORT_CASE_LIST = "import-case-list"
+MAKE_STUDIES_AVAILABLE = "make-studies-available"
 
-COMMANDS = [IMPORT_CANCER_TYPE, IMPORT_STUDY, IMPORT_STUDY_DATA, IMPORT_CASE_LIST, REMOVE_STUDY, REMOVE_SAMPLES, REMOVE_PATIENTS]
+COMMANDS = [IMPORT_CANCER_TYPE, IMPORT_STUDY, IMPORT_STUDY_DATA, IMPORT_CASE_LIST, REMOVE_STUDY, REMOVE_SAMPLES, REMOVE_PATIENTS, MAKE_STUDIES_AVAILABLE]
 
 # ------------------------------------------------------------------------------
 # sub-routines
@@ -79,11 +80,11 @@ def import_study(jvm_args, meta_filename):
     args.append("--noprogress") # don't report memory usage and % progress
     run_java(*args)
 
-def update_study_status(jvm_args, study_id):
+def update_study_status(jvm_args, study_id, status="AVAILABLE"):
     args = jvm_args.split(' ')
     args.append(UPDATE_STUDY_STATUS_CLASS)
     args.append(study_id)
-    args.append("AVAILABLE")
+    args.append(status)
     args.append("--noprogress") # don't report memory usage and % progress
     run_java(*args)
 
@@ -268,6 +269,9 @@ def process_command(jvm_args, command, meta_filename, data_filename, study_ids,
         import_data(jvm_args, meta_filename, data_filename, update_generic_assay_entity)
     elif command == IMPORT_CASE_LIST:
         import_case_list(jvm_args, meta_filename)
+    elif command == MAKE_STUDIES_AVAILABLE:
+        for study_id in study_ids.split(","):
+            update_study_status(jvm_args, study_id)
 
 def get_meta_filenames(data_directory):
     meta_filenames = [
@@ -466,8 +470,8 @@ def process_study_directory(jvm_args, study_directory, update_generic_assay_enti
     if study_meta_dictionary[study_meta_filename].get('add_global_case_list', 'false').lower() == 'true':
         add_global_case_list(jvm_args, study_id)
 
-    # enable study
-    update_study_status(jvm_args, study_id)
+    # the study stays UNAVAILABLE until the derived tables are rebuilt
+    return study_id
 
 def get_meta_filenames_by_type(data_directory) -> Dict[str, Tuple[str, Dict]]:
     """
@@ -546,8 +550,16 @@ def process_data_directory(jvm_args, data_directory, update_generic_assay_entity
     not_supported_meta_types = meta_file_type_to_meta_files.keys() - INCREMENTAL_UPLOAD_SUPPORTED_META_TYPES
     if not_supported_meta_types:
         raise NotImplementedError("These types do not support incremental upload: {}".format(", ".join(not_supported_meta_types)))
+    # the study is not removed first, so hide it here until the derived tables are rebuilt
+    study_id = next(iter(meta_file_type_to_meta_files.values()))[0][1]['cancer_study_identifier']
+    update_study_status(jvm_args, study_id, "UNAVAILABLE")
     import_incremental_data(jvm_args, data_directory, update_generic_assay_entity, meta_file_type_to_meta_files)
     update_case_lists_from_folder(jvm_args, data_directory, meta_file_type_to_meta_files)
+    return study_id
+
+def make_study_available(args, study_id):
+    # main() has already put the jar path into args.java_opts
+    update_study_status("-Dspring.profiles.active=dbcp " + args.java_opts, study_id)
 
 def usage():
     # TODO : replace this by usage string from interface()
@@ -617,6 +629,10 @@ def interface(args=None):
     remove_patients.add_argument('--patient_ids', type=str, required=True,
                         help='Patient ID(s). Comma separated, if multiple.')
 
+    make_studies_available = subparsers.add_parser('make-studies-available', parents=[parent_parser], add_help=False)
+    make_studies_available .add_argument('-ids', '--study_ids', type=str, required=True,
+                        help='Cancer Study ID(s) to mark AVAILABLE, comma separated')
+
     parser.add_argument('-c', '--command', type=str, required=False,
                         help='This argument is outdated. Please use the listed subcommands, without the -c flag. '
                         'Command for import. Allowed commands: ' + allowed_commands_csv)
@@ -697,10 +713,10 @@ def main(args):
 
     if args.data_directory is not None:
         check_dir(args.data_directory)
-        process_data_directory(jvm_args, args.data_directory, args.update_generic_assay_entity)
+        return process_data_directory(jvm_args, args.data_directory, args.update_generic_assay_entity)
     elif args.study_directory is not None:
         check_dir(args.study_directory)
-        process_study_directory(jvm_args, args.study_directory, args.update_generic_assay_entity)
+        return process_study_directory(jvm_args, args.study_directory, args.update_generic_assay_entity)
     else:
         check_args(args.command)
         check_files(args.meta_filename, args.data_filename)

scripts/importer/metaImport.py

@@ -129,6 +129,15 @@ def _print_need_to_update_derived_tables_warning():
         file=sys.stderr,
     )
 
+def _print_need_to_make_study_available_message(study_id):
+    print(
+        Color.BOLD +
+        'The study stays UNAVAILABLE until the derived tables are rebuilt. Then run:\n'
+        '    cbioportalImporter.py make-studies-available -ids ' + study_id + '\n' +
+        Color.END,
+        file=sys.stderr,
+    )
+
 if __name__ == '__main__':
     derive_tables_only = len(sys.argv) > 1 and sys.argv[1] == 'derive-tables'
     if derive_tables_only:
@@ -228,7 +237,7 @@ def _print_need_to_update_derived_tables_warning():
             if args.override_warning and not args.skip_db_import:
                 print(Color.BOLD + "Overriding Warnings. Importing study now" + Color.END, file=sys.stderr)
                 print("#" * 71 + "\n", file=sys.stderr)
-                cbioportalImporter.main(args)
+                study_id = cbioportalImporter.main(args)
                 exitcode = 0
                 # Rebuild derived tables after database-mutating operation
                 if not getattr(args, 'no_derive_tables', False):
@@ -243,15 +252,19 @@ def _print_need_to_update_derived_tables_warning():
                               "The database may be in an inconsistent state." +
                               Color.END, file=sys.stderr)
                         exitcode = 1
+                        _print_need_to_make_study_available_message(study_id)
+                    else:
+                        cbioportalImporter.make_study_available(args, study_id)
                 else:
                     _print_need_to_update_derived_tables_warning()
+                    _print_need_to_make_study_available_message(study_id)
             else:
                 print(Color.BOLD + "Warnings. Please fix your files or import with override warning option" + Color.END, file=sys.stderr)
                 print("#" * 71, file=sys.stderr)
         elif exitcode == 0 and not args.skip_db_import:
             print(Color.BOLD + "Everything looks good. Importing study now" + Color.END, file=sys.stderr)
             print("#" * 71 + "\n", file=sys.stderr)
-            cbioportalImporter.main(args)
+            study_id = cbioportalImporter.main(args)
             # Rebuild derived tables after database-mutating operation
             if not getattr(args, 'no_derive_tables', False):
                 print("\n")
@@ -265,8 +278,12 @@ def _print_need_to_update_derived_tables_warning():
                           "The database may be in an inconsistent state." +
                           Color.END, file=sys.stderr)
                     exitcode = 1
+                    _print_need_to_make_study_available_message(study_id)
+                else:
+                    cbioportalImporter.make_study_available(args, study_id)
             else:
                 _print_need_to_update_derived_tables_warning()
+                _print_need_to_make_study_available_message(study_id)
     except KeyboardInterrupt:
         print(Color.BOLD + "\nProcess interrupted. You will have to run this again to make sure study is completely loaded." + Color.END, file=sys.stderr)
         print("#" * 71, file=sys.stderr)

src/main/java/org/mskcc/cbio/portal/dao/DaoCancerStudy.java

@@ -295,7 +295,6 @@ public static void addCancerStudy(CancerStudy cancerStudy, boolean overwrite) th
         CancerStudy existing = getCancerStudyByStableId(stableId);
         if (existing!=null) {
             if (overwrite) {
-                //setStatus(Status.UNAVAILABLE, stableId);
                 deleteCancerStudy(existing.getInternalId());
             } else {
                 throw new DaoException("Cancer study " + stableId + "is already imported.");
@@ -458,7 +457,6 @@ public static void deleteAllRecords() throws DaoException {
     public static void deleteCancerStudy(String cancerStudyStableId) throws DaoException {
         CancerStudy study = getCancerStudyByStableId(cancerStudyStableId);
         if (study != null){
-            //setStatus(Status.UNAVAILABLE, cancerStudyStableId);
             deleteCancerStudy(study.getInternalId());
         }
     }
@@ -505,6 +503,10 @@ public static void deleteCancerStudy(int internalCancerStudyId) throws DaoExcept
         PreparedStatement pstmt = null;
         ResultSet rs = null;
         try {
+            // The cancer_study row is deleted last; hide the study first so the portal does not
+            // serve it while its data is partially deleted.
+            setStatus(Status.UNAVAILABLE, null, internalCancerStudyId);
+
             // check whether should delete generic assay meta profile by profile
             DaoGenericAssay.checkAndDeleteGenericAssayMetaInStudy(internalCancerStudyId);
 

src/test/resources/seed_mini.sql

@@ -545,7 +545,7 @@ INSERT INTO authorities (`email`, `authority`) values ('Dhorak@yahoo.com','ROLE_
 INSERT INTO authorities (`email`, `authority`) values ('Dhorak@yahoo.com','ROLE_MANAGER');
 
 INSERT INTO `info` (`db_schema_version`, `geneset_version`)
-VALUES ('3.0.0', 'test_geneset_version');
+VALUES ('3.0.1', 'test_geneset_version');
 
 --geneset
 INSERT INTO geneset (`id`, `genetic_entity_id`, `external_id`, `name`, `description`, `ref_link`) VALUES (1, 1, 'HGNC:1100', 'BRCA1', 'Breast cancer type 1 susceptibility protein', 'https://www.ncbi.nlm.nih.gov/gene/672');

tests/system_tests_import_data.py

@@ -31,16 +31,14 @@ def test_full_study_load(self, run_java, locate_jar):
         study_directory = 'test_data/study_es_0'
         args = ['--study_directory', study_directory]
         parsed_args = cbioportalImporter.interface(args)
-        cbioportalImporter.main(parsed_args)
+        study_id = cbioportalImporter.main(parsed_args)
 
         remove_study_call = call(*common_part, 'org.mskcc.cbio.portal.scripts.RemoveCancerStudy',
             'study_es_0', '--noprogress')
         create_study_call = call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportCancerStudy',
             f'{study_directory}/meta_study.txt', '--noprogress')
         clinical_sample_call = call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportClinicalData',
             '--meta', f'{study_directory}/meta_clinical_samples.txt', '--loadMode', 'bulkload', '--data', f'{study_directory}/data_clinical_samples.txt', '--noprogress')
-        make_study_available_call = call(*common_part, 'org.mskcc.cbio.portal.scripts.UpdateCancerStudy',
-            'study_es_0', 'AVAILABLE', '--noprogress')
         mol_profile_calls = [
                     call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportProfileData', '--meta', f'{study_directory}/meta_cna_log2.txt', '--loadMode', 'bulkload', '--update-info', 'False', '--data', f'{study_directory}/data_cna_log2.txt', '--noprogress'),
                     call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportProfileData', '--meta', f'{study_directory}/meta_expression_median.txt', '--loadMode', 'bulkload', '--update-info', 'False', '--data', f'{study_directory}/data_expression_median.txt', '--noprogress'),
@@ -80,14 +78,15 @@ def test_full_study_load(self, run_java, locate_jar):
             call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportSampleList', f'{study_directory}/case_lists/cases_sequenced.txt', '--noprogress'),
             call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportSampleList', f'{study_directory}/case_lists/cases_test.txt', '--noprogress'),
             call(*common_part, 'org.mskcc.cbio.portal.scripts.AddCaseList', 'study_es_0', 'all', '--noprogress'),
-            make_study_available_call,
         ])
 
         self.assertTrue(run_java.call_args_list.index(remove_study_call) < run_java.call_args_list.index(create_study_call))
         self.assertTrue(run_java.call_args_list.index(create_study_call) < run_java.call_args_list.index(clinical_sample_call))
         self.assertTrue(all(run_java.call_args_list.index(clinical_sample_call) <  run_java.call_args_list.index(mol_profile_call)
             for mol_profile_call in mol_profile_calls))
-        self.assertEqual(run_java.call_args_list[-1], make_study_available_call)
+        # The study is left UNAVAILABLE until the derived tables are rebuilt
+        self.assertNotIn('AVAILABLE', [c.args[-2] for c in run_java.call_args_list])
+        self.assertEqual(study_id, 'study_es_0')
 
 
     @mock.patch('importer.cbioportalImporter.locate_jar')
@@ -101,7 +100,7 @@ def test_incremental_load(self, run_java, locate_jar):
         data_directory = 'test_data/study_es_0_inc'
         args = ['--data_directory', data_directory]
         parsed_args = cbioportalImporter.interface(args)
-        cbioportalImporter.main(parsed_args)
+        study_id = cbioportalImporter.main(parsed_args)
 
         clinical_patient_call = call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportClinicalData', '--overwrite-existing',
             '--meta', f'{data_directory}/meta_clinical_patients.txt', '--loadMode', 'bulkload', '--data', f'{data_directory}/data_clinical_patients.txt', '--noprogress')
@@ -130,8 +129,12 @@ def test_incremental_load(self, run_java, locate_jar):
         seg_call = call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportCopyNumberSegmentData', '--overwrite-existing',
             '--meta', f'{data_directory}/meta_cna_hg19_seg.txt', '--loadMode', 'bulkload', '--data', f'{data_directory}/data_cna_hg19.seg', '--noprogress')
 
+        make_study_unavailable_call = call(*common_part, 'org.mskcc.cbio.portal.scripts.UpdateCancerStudy',
+            'study_es_0', 'UNAVAILABLE', '--noprogress')
+
         self.assertCountEqual(run_java.call_args_list, [
             call(*common_part, 'org.mskcc.cbio.portal.util.VersionUtil',),
+            make_study_unavailable_call,
             clinical_patient_call,
             clinical_sample_call,
             mutation_call,
@@ -149,6 +152,8 @@ def test_incremental_load(self, run_java, locate_jar):
 
         self.assertTrue(run_java.call_args_list.index(clinical_sample_call) < run_java.call_args_list.index(mutation_call))
         self.assertTrue(run_java.call_args_list.index(clinical_sample_call) < run_java.call_args_list.index(case_list_call))
+        self.assertEqual(run_java.call_args_list[1], make_study_unavailable_call)
+        self.assertEqual(study_id, 'study_es_0')
 
 
     @mock.patch('importer.cbioportalImporter.locate_jar')
@@ -165,11 +170,30 @@ def test_incremental_load_cna_discrete_long(self, run_java, locate_jar):
         cna_discrete_long_call = call(*common_part, 'org.mskcc.cbio.portal.scripts.ImportProfileData', '--overwrite-existing',
                 '--meta', f'{data_directory}/meta_cna_discrete_long.txt', '--loadMode', 'bulkload', '--update-info', 'False', '--data', f'{data_directory}/data_cna_discrete_long.txt', '--noprogress')
 
-        self.assertCountEqual(run_java.call_args_list, [
+        self.assertEqual(run_java.call_args_list, [
             call(*common_part, 'org.mskcc.cbio.portal.util.VersionUtil',),
+            call(*common_part, 'org.mskcc.cbio.portal.scripts.UpdateCancerStudy', 'study_es_0', 'UNAVAILABLE', '--noprogress'),
             cna_discrete_long_call,
             ])
 
+    @mock.patch('importer.cbioportalImporter.locate_jar')
+    @mock.patch('importer.cbioportalImporter.run_java')
+    def test_make_studies_available(self, run_java, locate_jar):
+        '''
+        Tests java commands that make-studies-available produces
+        '''
+        locate_jar.return_value = "test.jar"
+
+        args = ['make-studies-available', '--study_ids', 'STUDY1,STUDY2']
+        parsed_args = cbioportalImporter.interface(args)
+        cbioportalImporter.main(parsed_args)
+
+        self.assertEqual(run_java.call_args_list, [
+            call(*common_part, 'org.mskcc.cbio.portal.util.VersionUtil',),
+            call(*common_part, 'org.mskcc.cbio.portal.scripts.UpdateCancerStudy', 'STUDY1', 'AVAILABLE', '--noprogress'),
+            call(*common_part, 'org.mskcc.cbio.portal.scripts.UpdateCancerStudy', 'STUDY2', 'AVAILABLE', '--noprogress'),
+        ])
+
     @mock.patch('importer.cbioportalImporter.locate_jar')
     @mock.patch('importer.cbioportalImporter.run_java')
     def test_remove_samples(self, run_java, locate_jar):