diff --git a/apps/cli/utilities/s_code_project_updater/src/s_code_project_updater/commands.py b/apps/cli/utilities/s_code_project_updater/src/s_code_project_updater/commands.py index 576b3a55f471d58c0091ca1fc1d43c18c013cd08..f70b85c5c203dd6095863e9e3d36ce4bd1adfd9f 100644 --- a/apps/cli/utilities/s_code_project_updater/src/s_code_project_updater/commands.py +++ b/apps/cli/utilities/s_code_project_updater/src/s_code_project_updater/commands.py @@ -3,20 +3,22 @@ A module for updating properties (title, abstract, PI and coI) of a provided pro """ import argparse as ap import logging -import sys, os +import os +import sys import warnings from typing import List +from pymygdala import LogHandler, SendNRAOEvent +from schema.model import Author, Project +from schema.pstmodel import Person, UserAuthentication from sqlalchemy import exc as sa_exc, asc, desc - -from ._version import ___version___ as version +### pytest can find these modules just fine from command line from support.capo import get_my_capo_config from support.logging import get_console_logger, LOG_MESSAGE_FORMATTER -from pymygdala import LogHandler, SendNRAOEvent + +from schema import ArchiveDBSession +from ._version import ___version___ as version from .project_fetcher import ArchiveProjectFetcher -from schema import ArchiveDBSession, ScienceProduct -from schema.model import Author, Project -from schema.pstmodel import Person, UserAuthentication _APPLICATION_NAME = 's_code_project_updater' _LOG = get_console_logger(_APPLICATION_NAME, logging.DEBUG) @@ -32,18 +34,6 @@ _EPILOG = """Return values: 5: Update failed""" -def scode_project_from_args(namespace): - - ns_dict = namespace.__dict__ - project_code = ns_dict['project'] - title = ns_dict['title'] - abstract = ns_dict['abstract'] - investigators = ns_dict['investigators'] - - new_project = ArchiveProject(project_code=project_code, title=title, abstract=abstract, author_pst_ids=investigators) - return new_project - - class ScodeProjectUpdater: """ A class to bundle the operations involved with updating a project in the archive. @@ -66,13 +56,13 @@ class ScodeProjectUpdater: self.is_dry = False if not args_dict['investigators'] and not args_dict['title'] and not args_dict['abstract']: - self.get_minimal_args(args_dict) + self.set_minimum_properties_from_args(args_dict) return self.stored_project = None _LOG.debug(f'{self.args}') - self.sc_project = scode_project_from_args(self.args) + self.sc_project = self.scode_project_from_args(self.args) self.capo_config = get_my_capo_config(profile=self.args.profile) try: @@ -88,7 +78,25 @@ class ScodeProjectUpdater: 'in the archive. There should be only one PI and any number of ' 'unique CoIs on a project.', 2) - def get_minimal_args(self, args): + + def scode_project_from_args(self, args: ap.Namespace): + ''' turn the command-line arguments into a project ''' + + ns_dict = args.__dict__ + project_code = ns_dict['project'] + title = ns_dict['title'] + abstract = ns_dict['abstract'] + investigators = ns_dict['investigators'] + + new_project = ArchiveProject(project_code=project_code, + title=title, + abstract=abstract, + author_pst_ids=investigators) + return new_project + + + def set_minimum_properties_from_args(self, args): + ''' basic info needed for a fetch ''' self.project_code = args['project'] self.profile = args['profile'] self.is_dry = True @@ -120,12 +128,6 @@ class ScodeProjectUpdater: 'and what the project would look like after the changes.') self.parser = result - def capture_error(self, msg, code): - self.error_message = msg - self.code = code - _LOG.error(f'error message received: {self.error_message}; code = {self.code}') - return self.code, self.error_message - def exit_with_error(self, msg, code): """ On discovering we have an unresolvable condition the prevents us from proceeding with the @@ -134,7 +136,9 @@ class ScodeProjectUpdater: :param code: the exit code to accompany the error message :return: None """ - self.capture_error(msg, code) + self.error_message = msg + self.code = code + _LOG.error(f'error message received: {self.error_message}; code = {self.code}') _LOG.error(msg) self.parser.print_help() sys.exit(code) @@ -192,8 +196,9 @@ class ScodeProjectUpdater: and person_id :return: None """ - old_investigators = self.get_projects_current_investigators() - # if any of the new investigators already exists, use the old author_id rather than making a new author + + # if any of the new investigators already exists, + # use the old author_id rather than making a new author is_pi = True num_expected = len(new_investigators) num_changed = 0 @@ -220,7 +225,7 @@ class ScodeProjectUpdater: if num_changed < num_expected: _LOG.error(f'{num_changed} of {num_expected} investigators were NOT set') - raise Exception('incomplete investigator update') + raise UpdateException('incomplete investigator update') def print_project(self): """ @@ -258,9 +263,14 @@ class ScodeProjectUpdater: except AttributeError: return False - def update_project(self): + def update_project(self) -> Project: + ''' + Where the magic happens: update aspects of the project + (including authors) according to the arguments passed in + :return: + ''' + fetcher = ArchiveProjectFetcher(self.profile) if self.is_fetch_only(): - fetcher = ArchiveProjectFetcher(self.profile) self.project = fetcher.fetch_project(self.project_code) output = fetcher.build_project_info() try: @@ -286,13 +296,9 @@ class ScodeProjectUpdater: if self.stored_project is None: self.exit_with_error('No project found for the project_code provided', 3) - # is this an ALMA project? - self.product = self.archive_context.session.query(ScienceProduct) \ - .filter(ScienceProduct.project == self.stored_project) \ - .first() - external_system = self.product.external_system - if str(external_system).startswith("ALMA"): - raise ValueError(f'{self.stored_project.project_code} is an ALMA project; update not permitted') + if fetcher.is_alma: + raise ValueError(f'{self.stored_project.project_code} ' + f'is an ALMA project; update not permitted') if self.args.investigators: proposed_investigators = self.get_pst_users(self.args.investigators) @@ -314,9 +320,9 @@ class ScodeProjectUpdater: if not self.args.dry: if not self.is_fetch_only(): self.archive_context.session.commit() - _LOG.info(f'Changes committed') + _LOG.info('Changes committed') elif not self.is_fetch_only(): - _LOG.info(f'Successful dry run; this would have updated the project') + _LOG.info('Successful dry run; this would have updated the project') self.print_project() return self.stored_project @@ -389,9 +395,9 @@ class ArchiveProject: def set_alma(self, is_alma): self.is_alma = is_alma - def make_args(self, isDry): + def make_args(self, is_dry): args = [] - if isDry: + if is_dry: args.append('-d') args.append('-C') @@ -413,7 +419,8 @@ class ArchiveProject: @staticmethod def from_schema_project(project: Project, is_alma: bool): - to_return = ArchiveProject(project.project_code, project.title, project.abstract, project.authors) + to_return = ArchiveProject(project.project_code, project.title, + project.abstract, project.authors) to_return.set_alma(is_alma) return to_return @@ -438,6 +445,9 @@ class ArchiveProject: return new_project.investigators +class UpdateException(Exception): + ''' throw this if there is trouble during the update ''' + pass def main(**kwargs): """ @@ -454,5 +464,3 @@ def main(**kwargs): if __name__ == '__main__': main() - - diff --git a/apps/cli/utilities/s_code_project_updater/src/s_code_project_updater/project_fetcher.py b/apps/cli/utilities/s_code_project_updater/src/s_code_project_updater/project_fetcher.py index b66d24241c1ca3af45214782ff968dcfeb512357..81fd02e0777247f88801d9303d80b7b1f73ff6c6 100644 --- a/apps/cli/utilities/s_code_project_updater/src/s_code_project_updater/project_fetcher.py +++ b/apps/cli/utilities/s_code_project_updater/src/s_code_project_updater/project_fetcher.py @@ -2,10 +2,15 @@ import logging import sys import warnings +from sqlalchemy import exc as sa_exc, asc, desc + +## pytest can't find these +from schema import ArchiveDBSession, create_session, ExecutionBlock +from schema.model import Project, Author from support.capo import get_my_capo_config from support.logging import get_console_logger -from schema import ArchiveDBSession, Project, Author, ScienceProduct -from sqlalchemy import exc as sa_exc, asc, desc + +from . import Telescope _APPLICATION_NAME = 'project_fetcher' _LOG = get_console_logger(_APPLICATION_NAME, logging.DEBUG) @@ -18,50 +23,55 @@ class ArchiveProjectFetcher: def __init__(self, profile): self.capo_config = get_my_capo_config(profile=profile) try: - self.archive_context = ArchiveDBSession('SDM', profile=self.capo_config.profile) + self.archive_context = create_session('SDM', + profile=self.capo_config.profile) self.pst_context = ArchiveDBSession('PST', profile=self.capo_config.profile) except KeyError as k_ex: _LOG.error(f'An error occurred while creating a db context: {k_ex}') sys.exit(1) - def fetch_project(self, project_code): - with warnings.catch_warnings(), self.archive_context, self.pst_context: + def fetch_project(self, project_code: str): + + with warnings.catch_warnings(): # , self.archive_session, \ + # self.pst_session: # Suppress SQLAlchemy warnings warnings.simplefilter("ignore", category=sa_exc.SAWarning) """ Return the project specified by the input arguments, if it exists. :return: the first Project we found with the project code passed in """ - self.project = self.archive_context.session.query(Project) \ + self.project = self.archive_context.query(Project) \ .filter(Project.project_code == project_code) \ .first() - self.abstract = self.project.abstract + if self.project is None: + raise AttributeError(f'project {project_code} not found') + self.abstract = self.project.abstract self.is_alma = self._is_alma() - self.authors = self._get_investigators() - self.detachable_author_list = self._get_detached_authors() - self.project_info = self.build_project_info() return self.project def build_project_info(self): + ''' assemble the output ''' output = [] output.append(f'Title: {self.project.title}') output.append(f'Abstract: {self.project.abstract}') if self._is_alma(): return - investigator_list = self.authors - # investigator_list = self._get_investigators() - # We want the PI's pst_person_id followed by the CoIs' pst_person_ids in numeric order. + investigator_list = self.project.authors + if not investigator_list: + raise ValueError(f'no authors found for {self.project.project_code}') + + # We want the PI's pst_person_id followed by the CoIs' + # pst_person_ids in numeric order. # ALMA authors, however, do not have pst_person_ids - pi = investigator_list[0] + project_pi = investigator_list[0] coi_pst_ids = [int(coi.pst_person_id) for coi in investigator_list[1:]] - # TODO: should not need to sort; query does that coi_pst_ids = sorted(coi_pst_ids) - author_pst_ids = [int(pi.pst_person_id)] + author_pst_ids = [int(project_pi.pst_person_id)] [author_pst_ids.append(id) for id in coi_pst_ids] authors_to_print = [str(id) for id in author_pst_ids] id_list = ' '.join(authors_to_print) @@ -71,58 +81,30 @@ class ArchiveProjectFetcher: def _get_investigators(self): """ - Get a list of investigators associated with this project, with PI(s) as first element(s) of the list + Get a list of investigators associated with this project, + with PI(s) as first element(s) of the list - :return: a list of investigators associated with the project code passed in, ordered with - the PI(s) first + :return: a list of investigators associated with the project code passed in, + ordered with the PI(s) first """ - investigators_list = self.archive_context.session.query(Author) \ - .filter(Author.project == self.project) \ - .order_by(desc(Author.is_pi), asc(Author.pst_person_id)) \ - .all() + with warnings.catch_warnings(): + # Suppress SQLAlchemy warnings + warnings.simplefilter("ignore", category=sa_exc.SAWarning) + + investigators_list = self.archive_context.query(Author) \ + .filter(Author.project == self.project) \ + .order_by(desc(Author.is_pi), asc(Author.pst_person_id)) \ + .all() return investigators_list def _is_alma(self): - with warnings.catch_warnings(), self.archive_context, self.pst_context: + ''' is this an alma project? ''' + with warnings.catch_warnings(): # Suppress SQLAlchemy warnings warnings.simplefilter("ignore", category=sa_exc.SAWarning) - self.product = self.archive_context.session.query(ScienceProduct)\ - .filter(ScienceProduct.project == self.project)\ - .first() - external_system = self.product.external_system - return str(external_system).startswith("ALMA") - - def _get_detached_authors(self): - return [DetachedAuthor(author) for author in self.authors] - -if __name__ == '__main__': - fetched = ArchiveProjectFetcher("nmtest").fetch_project('SK0442') - authors = fetched.authors - assert 4 == len(fetched.authors) - _LOG.debug("looks ok") - - -class DetachedAuthor: - def __init__(self, author:Author): - self.author_id = author.author_id - self.username = author.username - self.firstname = author.firstname - self.lastname = author.lastname - self.pst_person_id = author.pst_person_id - self.is_pi = author.is_pi - # self.project_code = author.project_code - - def __eq__(self, other): - if type(other) is not type(self): - return False - - if other.author_id == self.author_id: - if other.pst_person_id == self.pst_person_id: - if other.lastname == self.lastname: - return other.firstname == self.firstname - - return False - - def __repr__(self): - return f'{self.firstname} {self.lastname}: {self.author_id}, {self.pst_person_id}' \ No newline at end of file + exec_block = self.archive_context.query(ExecutionBlock) \ + .filter(ExecutionBlock.project == self.project) \ + .filter(ExecutionBlock.telescope == Telescope.ALMA.value) \ + .first() + return exec_block is not None diff --git a/apps/cli/utilities/s_code_project_updater/test/test_updater.py b/apps/cli/utilities/s_code_project_updater/test/test_updater.py index 1bb9c177047838f75e67cf56e127c889dfbea5b0..89cb5bb3cca603db1c74723f1e1c1b0da88f0ee3 100644 --- a/apps/cli/utilities/s_code_project_updater/test/test_updater.py +++ b/apps/cli/utilities/s_code_project_updater/test/test_updater.py @@ -1,14 +1,17 @@ ''' unit/regression tests for s_code_project_updater ''' + import logging import os import unittest - import pytest from astropy.time import Time -from shared.schema.src.schema import create_session, Project -from shared.support.src.support.logging import get_console_logger +from s_code_project_updater.commands import \ + ScodeProjectUpdater +from schema import create_session, Project +from schema.pstmodel import Session +from support.logging import get_console_logger from .test_projects import AlmaTestProject, ScodeTestProject, ScienceTestProject @@ -20,11 +23,173 @@ class UpdaterTestCase(unittest.TestCase): def setUpClass(cls) -> None: os.environ['CAPO_PROFILE'] = 'local' - # @pytest.mark.skip('test_create_scode_test_project') - def test_create_scode_test_project(self): + ### making sure fake-project creation works ### + + def test_create_scode_project(self): + session = None + project = None + try: + session = create_session('SDM') + except Exception as exc: + pytest.fail(f'failed to create session: {exc}') + + project = self.initialize_scode_test_project(session) + self.assertIsNotNone(project) + expected = ScodeTestProject() + self.assertEqual(expected.project_code, + project.project_code, + f'expected {expected.project_code}; got {project.project_code}') + num_authors_expected = len(expected.authors) + actual = len(project.authors) + self.assertEqual(num_authors_expected, actual, + f'expecting {num_authors_expected} authors but got {actual}') + + finally: + if project is not None: + try: + session.delete(project) + except Exception as exc: + _LOG.warning(f'delete failure: {exc}') + if session is not None: + session.close() + + def test_create_science_project(self): + session = None + project = None + try: + session = create_session('SDM') + except Exception as exc: + pytest.fail(f'failed to create session: {exc}') + + project = self.initialize_science_test_project(session) + self.assertIsNotNone(project) + expected = ScodeTestProject() + self.assertEqual(expected.project_code, + project.project_code, + f'expected {expected.project_code}; got {project.project_code}') + num_authors_expected = len(expected.authors) + actual = len(project.authors) + self.assertEqual(num_authors_expected, actual, + f'expecting {num_authors_expected} authors but got {actual}') + + finally: + if project is not None: + try: + session.delete(project) + except Exception as exc: + _LOG.warning(f'delete failure: {exc}') + if session is not None: + session.close() + + def test_create_alma_project(self): + session = None + project = None + try: + session = create_session('SDM') + except Exception as exc: + pytest.fail(f'failed to create session: {exc}') + + project = self.initialize_alma_test_project(session) + self.assertIsNotNone(project) + expected = ScodeTestProject() + self.assertEqual(expected.project_code, + project.project_code, + f'expected {expected.project_code}; got {project.project_code}') + num_authors_expected = len(expected.authors) + actual = len(project.authors) + self.assertEqual(num_authors_expected, actual, + f'expecting {num_authors_expected} authors but got {actual}') + + finally: + if project is not None: + try: + session.delete(project) + except Exception as exc: + _LOG.warning(f'delete failure: {exc}') + if session is not None: + session.close() + + ### THE MEAT ### + def test_dry_run_does_not_update(self): + session = None + project = None + try: + session = create_session('SDM') + except Exception as exc: + pytest.fail(f'failed to create session: {exc}') + + project = self.initialize_scode_test_project(session) + args = [ + '-C', project.project_code, + '-P', os.environ['profile'], + '-T', 'this is the new title', + '-dry' + ] + scode_updater = ScodeProjectUpdater(args) + updated = scode_updater.update_project() + self.assertIsNone(updated, 'project should not have been updated') + + after = self.initialize_scode_test_project(session) + self.assertEqual(after.project_code, + project.project_code, + f'expected {after.project_code}; got {project.project_code}') + num_authors_expected = len(after.authors) + actual = len(project.authors) + self.assertEqual(num_authors_expected, actual, + f'expecting {num_authors_expected} authors but got {actual}') + + finally: + if project is not None: + try: + session.delete(project) + except Exception as exc: + _LOG.warning(f'delete failure: {exc}') + if session is not None: + session.close() + + def test_alma_project_not_updated(self): session = None + project = None try: session = create_session('SDM') + except Exception as exc: + pytest.fail(f'failed to create session: {exc}') + + project = self.initialize_alma_project(session) + pst_ids = self.get_author_pst_ids(project) + args = [ + '-C', project.project_code, + '-P', os.environ['profile'], + '-I', pst_ids[1:], + '-T', 'this is the new title', + ] + scode_updater = ScodeProjectUpdater(args) + updated = scode_updater.update_project() + self.assertIsNone(updated, 'project should not have been updated') + + after = self.initialize_scode_test_project(session) + self.assertEqual(after.project_code, + project.project_code, + f'expected {after.project_code}; got {project.project_code}') + num_authors_expected = len(after.authors) + actual = len(project.authors) + self.assertEqual(num_authors_expected, actual, + f'expecting {num_authors_expected} authors but got {actual}') + + finally: + if project is not None: + try: + session.delete(project) + except Exception as exc: + _LOG.warning(f'delete failure: {exc}') + if session is not None: + session.close() + + + ### UTILITIES ### + + def initialize_scode_test_project(self, session: Session): + try: project = ScodeTestProject() result = None @@ -47,22 +212,15 @@ class UpdaterTestCase(unittest.TestCase): # insert fake project try: session.add(project) + return project except Exception as exc: pytest.fail(f'insert failure: {exc}') except Exception as exc: pytest.fail(f'failed to create session: {exc}') - finally: - if session is not None: - # 1. delete fake project, then - session.close() - - # @pytest.mark.skip('test_create_science_project') - def test_create_science_project(self): - session = None + def initialize_science_test_project(self, session: Session): try: - session = create_session('SDM') project = ScienceTestProject() result = None @@ -85,20 +243,15 @@ class UpdaterTestCase(unittest.TestCase): # insert fake project try: session.add(project) + return project except Exception as exc: pytest.fail(f'insert failure: {exc}') except Exception as exc: pytest.fail(f'failed to create session: {exc}') - finally: - if session is not None: - # 1. delete fake project, then - session.close() - def test_create_alma_project(self): - session = None + def initialize_alma_project(self, session: Session): try: - session = create_session('SDM') project = AlmaTestProject() result = None @@ -121,17 +274,12 @@ class UpdaterTestCase(unittest.TestCase): # insert fake project try: session.add(project) + return project except Exception as exc: pytest.fail(f'insert failure: {exc}') except Exception as exc: pytest.fail(f'failed to create session: {exc}') - finally: - if session is not None: - # 1. delete fake project, then - session.close() - - ### UTILITIES ### @staticmethod def get_author_pst_ids(project: Project): @@ -162,42 +310,6 @@ class UpdaterTestCase(unittest.TestCase): elapsed_time = now - release_time return elapsed_time > 0 - def template(self): - ''' just for copying & pasting ''' - session = None - try: - session = create_session('SDM') - try: - # see if fake project already in DB - pass - except Exception as exc: - pytest.fail(f'query failure: {exc}') - - # if it's there, - try: - # delete fake project - pass - except Exception as exc: - pytest.fail(f'delete failure: {exc}') - - try: - # insert fake project - pass - except Exception as exc: - pytest.fail(f'insert failure: {exc}') - - try: - # exercise feature - pass - except Exception as exc: - pytest.fail(f'TODO: {exc}') - - except Exception as exc: - pytest.fail(f'failed to create session: {exc}') - finally: - if session is not None: - # 1. delete fake project, then - session.close() def debug_multiple_fetch_create_session(self): ''' for debugging connection issues;