From 9fd954e0ac9e6135549fd809ea6f3b771fa23f29 Mon Sep 17 00:00:00 2001 From: burnout87 Date: Wed, 2 Oct 2024 14:16:32 +0200 Subject: [PATCH 01/13] in case local resolver fails, fallback to the external resolver --- cdci_data_analysis/analysis/drupal_helper.py | 44 +++++++++++++++----- cdci_data_analysis/flask_app/app.py | 11 +++-- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/cdci_data_analysis/analysis/drupal_helper.py b/cdci_data_analysis/analysis/drupal_helper.py index ded4b7875..9dc6378c5 100644 --- a/cdci_data_analysis/analysis/drupal_helper.py +++ b/cdci_data_analysis/analysis/drupal_helper.py @@ -21,6 +21,7 @@ from enum import Enum, auto from astropy.coordinates import SkyCoord, Angle from astropy import units as u +import xml.etree.ElementTree as ET from cdci_data_analysis.analysis import tokenHelper from ..analysis.exceptions import RequestNotUnderstood, InternalError, RequestNotAuthorized @@ -551,11 +552,14 @@ def post_content_to_gallery(decoded_token, if update_astro_entity: auto_update = kwargs.pop('auto_update', 'False') == 'True' if auto_update is True: - name_resolver_url = disp_conf.name_resolver_url + local_name_resolver_url = disp_conf.local_name_resolver_url + external_name_resolver_url = disp_conf.external_name_resolver_url entities_portal_url = disp_conf.entities_portal_url - resolved_obj = resolve_name(name_resolver_url=name_resolver_url, + resolved_obj = resolve_name(local_name_resolver_url=local_name_resolver_url, + external_name_resolver_url=external_name_resolver_url, entities_portal_url=entities_portal_url, - name=src_name) + name=src_name, + sentry_dsn=sentry_dsn) if resolved_obj is not None: msg = '' if 'message' in resolved_obj: @@ -1488,11 +1492,11 @@ def check_matching_coords(source_1_name, source_1_coord_ra, source_1_coord_dec, return False -def resolve_name(name_resolver_url: str, entities_portal_url: str = None, name: str = None): +def resolve_name(local_name_resolver_url: str, external_name_resolver_url: str, entities_portal_url: str = None, name: str = None, sentry_dsn=None): resolved_obj = {} if name is not None: quoted_name = urllib.parse.quote(name.strip()) - res = requests.get(name_resolver_url.format(quoted_name)) + res = requests.get(local_name_resolver_url.format(quoted_name)) if res.status_code == 200: returned_resolved_obj = res.json() if 'success' in returned_resolved_obj: @@ -1513,12 +1517,32 @@ def resolve_name(name_resolver_url: str, entities_portal_url: str = None, name: logger.info(f"resolution of the object {name} unsuccessful") resolved_obj['message'] = f'{name} could not be resolved' else: - logger.warning(f"there seems to be some problem in completing the request for the resolution of the object: {name}\n" - f"the request lead to the error {res.text}, " + logger.warning("There seems to be some problem in completing the request for the resolution of the object" + f" \"{name}\" using the local resolver.\n" + f"The request lead to the error {res.text}, " "this might be due to an error in the url or the service " - "requested is currently not available, " - "please check your request and try to issue it again") - raise InternalError('issue when performing a request to the local resolver', + "requested is currently not available. The external resolver will be used.") + if sentry_dsn is not None: + sentry.capture_message(f'Issue in resolving the object {name} using the local resolver\n{res.text}') + res = requests.get(external_name_resolver_url.format(quoted_name)) + if res.status_code == 200: + root = ET.fromstring(res.text) + resolver_tag = root.find('.//Resolver') + if resolver_tag is not None: + ra_tag = resolver_tag.find('.//jradeg') + resolved_obj['RA'] = float(ra_tag.text) + + dec_tag = resolver_tag.find('.//jdedeg') + resolved_obj['DEC'] = float(dec_tag.text) + else: + logger.warning("There seems to be some problem in completing the request for the resolution of the object" + f" \"{name}\" using the external resolver.\n" + f"The request lead to the error {res.text}, " + "this might be due to an error in the url or the service " + "requested is currently not available. The object could not be resolved.") + if sentry_dsn is not None: + sentry.capture_message(f'Issue in resolving the object {name} using the external resolver\n{res.text}') + raise InternalError('issue when performing a request to the external resolver', status_code=500, payload={'drupal_helper_error_message': res.text}) return resolved_obj diff --git a/cdci_data_analysis/flask_app/app.py b/cdci_data_analysis/flask_app/app.py index 8627ac8b0..968b600be 100644 --- a/cdci_data_analysis/flask_app/app.py +++ b/cdci_data_analysis/flask_app/app.py @@ -650,12 +650,17 @@ def resolve_name(): name = par_dic.get('name', None) - name_resolver_url = app_config.name_resolver_url + local_name_resolver_url = app_config.local_name_resolver_url + external_name_resolver_url = app_config.external_name_resolver_url entities_portal_url = app_config.entities_portal_url - resolve_object = drupal_helper.resolve_name(name_resolver_url=name_resolver_url, + sentry_dsn = sentry.sentry_url + + resolve_object = drupal_helper.resolve_name(local_name_resolver_url=local_name_resolver_url, + external_name_resolver_url=external_name_resolver_url, entities_portal_url=entities_portal_url, - name=name) + name=name, + sentry_dsn=sentry_dsn) return resolve_object From d0b647bde37182aca4459e6bb72bfbb546ee340f Mon Sep 17 00:00:00 2001 From: burnout87 Date: Wed, 2 Oct 2024 14:16:42 +0200 Subject: [PATCH 02/13] extended configuration --- cdci_data_analysis/configurer.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/cdci_data_analysis/configurer.py b/cdci_data_analysis/configurer.py index d35e14896..e6591a965 100644 --- a/cdci_data_analysis/configurer.py +++ b/cdci_data_analysis/configurer.py @@ -260,7 +260,10 @@ def __init__(self, cfg_dict, origin=None): disp_dict.get('product_gallery_options', {}).get('product_gallery_secret_key', None), disp_dict.get('product_gallery_options', {}).get('product_gallery_timezone', "Europe/Zurich"), - disp_dict.get('product_gallery_options', {}).get('name_resolver_url', 'https://resolver-prod.obsuks1.unige.ch/api/v1.1/byname/{}'), + disp_dict.get('product_gallery_options', {}).get('local_name_resolver_url', + 'https://resolver-prod.obsuks1.unige.ch/api/v1.1/byname/{}'), + disp_dict.get('product_gallery_options', {}).get('external_name_resolver_url', + 'http://cdsweb.u-strasbg.fr/cgi-bin/nph-sesame/-oxp/NSV?{}'), disp_dict.get('product_gallery_options', {}).get('entities_portal_url', 'http://cdsportal.u-strasbg.fr/?target={}'), disp_dict.get('product_gallery_options', {}).get('converttime_revnum_service_url', 'https://www.astro.unige.ch/mmoda/dispatch-data/gw/timesystem/api/v1.0/converttime/UTC/{}/REVNUM'), disp_dict.get('renku_options', {}).get('renku_gitlab_repository_url', None), @@ -338,7 +341,8 @@ def set_conf_dispatcher(self, product_gallery_url, product_gallery_secret_key, product_gallery_timezone, - name_resolver_url, + local_name_resolver_url, + external_name_resolver_url, entities_portal_url, converttime_revnum_service_url, renku_gitlab_repository_url, @@ -389,7 +393,8 @@ def set_conf_dispatcher(self, self.product_gallery_url = product_gallery_url self.product_gallery_secret_key = product_gallery_secret_key self.product_gallery_timezone = product_gallery_timezone - self.name_resolver_url = name_resolver_url + self.local_name_resolver_url = local_name_resolver_url + self.external_name_resolver_url = external_name_resolver_url self.entities_portal_url = entities_portal_url self.converttime_revnum_service_url = converttime_revnum_service_url self.renku_gitlab_repository_url = renku_gitlab_repository_url From 46af8a24cb7ffb9c8d32ed28bd5aa4dfa6397f03 Mon Sep 17 00:00:00 2001 From: burnout87 Date: Wed, 2 Oct 2024 17:30:28 +0200 Subject: [PATCH 03/13] handling not resolvable url --- cdci_data_analysis/analysis/drupal_helper.py | 123 ++++++++++++------- 1 file changed, 81 insertions(+), 42 deletions(-) diff --git a/cdci_data_analysis/analysis/drupal_helper.py b/cdci_data_analysis/analysis/drupal_helper.py index 9dc6378c5..3a3beb9a2 100644 --- a/cdci_data_analysis/analysis/drupal_helper.py +++ b/cdci_data_analysis/analysis/drupal_helper.py @@ -1496,55 +1496,94 @@ def resolve_name(local_name_resolver_url: str, external_name_resolver_url: str, resolved_obj = {} if name is not None: quoted_name = urllib.parse.quote(name.strip()) - res = requests.get(local_name_resolver_url.format(quoted_name)) + local_name_resolver_url_formatted = local_name_resolver_url.format(quoted_name) + try: + res = requests.get(local_name_resolver_url_formatted) + if res.status_code == 200: + returned_resolved_obj = res.json() + if 'success' in returned_resolved_obj: + resolved_obj['name'] = name.replace('_', ' ') + if returned_resolved_obj['success']: + logger.info(f"object {name} successfully resolved") + if 'ra' in returned_resolved_obj: + resolved_obj['RA'] = float(returned_resolved_obj['ra']) + if 'dec' in returned_resolved_obj: + resolved_obj['DEC'] = float(returned_resolved_obj['dec']) + if 'object_ids' in returned_resolved_obj: + resolved_obj['object_ids'] = returned_resolved_obj['object_ids'] + if 'object_type' in returned_resolved_obj: + resolved_obj['object_type'] = returned_resolved_obj['object_type'] + resolved_obj['entity_portal_link'] = entities_portal_url.format(quoted_name) + resolved_obj['message'] = f'{name} successfully resolved' + elif not returned_resolved_obj['success']: + logger.info(f"resolution of the object {name} unsuccessful") + resolved_obj['message'] = f'{name} could not be resolved' + else: + logger.warning("There seems to be some problem in completing the request for the resolution of the object" + f" \"{name}\" using the local resolver.\n" + f"The request lead to the error {res.text}, " + "this might be due to an error in the url or the service " + "requested is currently not available. The external resolver will be used.") + if sentry_dsn is not None: + sentry.capture_message(f'Failed to resolve object "{name}" using the local resolver. ' + f'URL: {local_name_resolver_url_formatted} ' + f'Status Code: {res.status_code} ' + f'Response: {res.text}') + except (ConnectionError, + requests.exceptions.ConnectionError, + requests.exceptions.Timeout) as e: + logger.warning(f'An exception occurred while trying to resolve the object "{name}" using the local resolver. ' + f'using the url: {local_name_resolver_url_formatted}. Exception details: {str(e)}') + if sentry_dsn is not None: + sentry.capture_message(f'An exception occurred while trying to resolve the object "{name}" using the local resolver. ' + f'URL: {local_name_resolver_url_formatted} ' + f"Exception details: {str(e)}") + res = requests.get(external_name_resolver_url.format(quoted_name)) if res.status_code == 200: - returned_resolved_obj = res.json() - if 'success' in returned_resolved_obj: - resolved_obj['name'] = name.replace('_', ' ') - if returned_resolved_obj['success']: - logger.info(f"object {name} successfully resolved") - if 'ra' in returned_resolved_obj: - resolved_obj['RA'] = float(returned_resolved_obj['ra']) - if 'dec' in returned_resolved_obj: - resolved_obj['DEC'] = float(returned_resolved_obj['dec']) - if 'object_ids' in returned_resolved_obj: - resolved_obj['object_ids'] = returned_resolved_obj['object_ids'] - if 'object_type' in returned_resolved_obj: - resolved_obj['object_type'] = returned_resolved_obj['object_type'] - resolved_obj['entity_portal_link'] = entities_portal_url.format(quoted_name) - resolved_obj['message'] = f'{name} successfully resolved' - elif not returned_resolved_obj['success']: - logger.info(f"resolution of the object {name} unsuccessful") + root = ET.fromstring(res.text) + resolved_obj['name'] = name.replace('_', ' ') + resolver_tag = root.find('.//Resolver') + if resolver_tag is not None: + ra_tag = resolver_tag.find('.//jradeg') + dec_tag = resolver_tag.find('.//jdedeg') + if ra_tag is None or dec_tag is None: + info_tag = root.find('.//INFO') resolved_obj['message'] = f'{name} could not be resolved' + if info_tag is not None: + message_info = info_tag.text + resolved_obj['message'] += f': {message_info}' + else: + resolved_obj['RA'] = float(ra_tag.text) + resolved_obj['DEC'] = float(dec_tag.text) + resolved_obj['entity_portal_link'] = entities_portal_url.format(quoted_name) + else: + warning_msg = ("There seems to be some problem in completing the request for the resolution of the object" + f" \"{name}\" using the external resolver.") + resolved_obj['message'] = f'{name} could not be resolved' + info_tag = root.find('.//INFO') + if info_tag is not None: + warning_msg += (f"The request lead to the error {info_tag.text}, " + "this might be due to an error in the name of the object that ha been provided.") + resolved_obj['message'] += f': {info_tag.text}' + logger.warning(warning_msg) + if sentry_dsn is not None: + sentry.capture_message(f'Failed to resolve object "{name}" using the remote resolver. ' + f'URL: {local_name_resolver_url.format(quoted_name)} ' + f'Status Code: {res.status_code} ' + f'Response: {res.text}' + f"Info returned from the resolver: {resolved_obj['message']}") else: logger.warning("There seems to be some problem in completing the request for the resolution of the object" - f" \"{name}\" using the local resolver.\n" + f" \"{name}\" using the external resolver.\n" f"The request lead to the error {res.text}, " "this might be due to an error in the url or the service " - "requested is currently not available. The external resolver will be used.") + "requested is currently not available. The object could not be resolved.") if sentry_dsn is not None: - sentry.capture_message(f'Issue in resolving the object {name} using the local resolver\n{res.text}') - res = requests.get(external_name_resolver_url.format(quoted_name)) - if res.status_code == 200: - root = ET.fromstring(res.text) - resolver_tag = root.find('.//Resolver') - if resolver_tag is not None: - ra_tag = resolver_tag.find('.//jradeg') - resolved_obj['RA'] = float(ra_tag.text) - - dec_tag = resolver_tag.find('.//jdedeg') - resolved_obj['DEC'] = float(dec_tag.text) - else: - logger.warning("There seems to be some problem in completing the request for the resolution of the object" - f" \"{name}\" using the external resolver.\n" - f"The request lead to the error {res.text}, " - "this might be due to an error in the url or the service " - "requested is currently not available. The object could not be resolved.") - if sentry_dsn is not None: - sentry.capture_message(f'Issue in resolving the object {name} using the external resolver\n{res.text}') - raise InternalError('issue when performing a request to the external resolver', - status_code=500, - payload={'drupal_helper_error_message': res.text}) + sentry.capture_message(f'Failed to resolve object "{name}" using the remote resolver. ' + f'URL: {local_name_resolver_url.format(quoted_name)} ' + f'Status Code: {res.status_code} ' + f'Response: {res.text}') + resolved_obj['message'] = f'{name} could not be resolved: {res.text}' return resolved_obj From 6b798476f045c1dbfefcfc4a1f62891e1add805f Mon Sep 17 00:00:00 2001 From: burnout87 Date: Wed, 2 Oct 2024 17:31:06 +0200 Subject: [PATCH 04/13] merging --- cdci_data_analysis/pytest_fixtures.py | 27 +++++++++++++++ tests/conftest.py | 2 ++ tests/test_server_basic.py | 47 +++++++++++++++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/cdci_data_analysis/pytest_fixtures.py b/cdci_data_analysis/pytest_fixtures.py index 85649a536..c3c4000c4 100644 --- a/cdci_data_analysis/pytest_fixtures.py +++ b/cdci_data_analysis/pytest_fixtures.py @@ -611,6 +611,27 @@ def dispatcher_test_conf_with_gallery_fn(dispatcher_test_conf_fn): yield fn +@pytest.fixture +def dispatcher_test_conf_with_vo_options_fn(dispatcher_test_conf_fn): + fn = "test-dispatcher-conf-with-vo-options.yaml" + + with open(fn, "w") as f: + with open(dispatcher_test_conf_fn) as f_default: + f.write(f_default.read()) + + f.write('\n vo_options:' + '\n vo_mysql_pg_host: "localhost"' + '\n vo_mysql_pg_user: "user"' + '\n vo_mysql_pg_password: "password"' + '\n vo_mysql_pg_db: "database"' + '\n vo_psql_pg_host: "localhost"' + '\n vo_psql_pg_user: "user"' + '\n vo_psql_pg_password: "password"' + '\n vo_psql_pg_db: "database"') + + yield fn + + @pytest.fixture def dispatcher_test_conf_with_matrix_options_fn(dispatcher_test_conf_fn): fn = "test-dispatcher-conf-with-matrix-options.yaml" @@ -708,10 +729,16 @@ def dispatcher_test_conf_with_gallery(dispatcher_test_conf_with_gallery_fn): yield yaml.load(open(dispatcher_test_conf_with_gallery_fn), Loader=yaml.SafeLoader)['dispatcher'] +@pytest.fixture +def dispatcher_test_conf_with_vo_options(dispatcher_test_conf_with_vo_options_fn): + yield yaml.load(open(dispatcher_test_conf_with_vo_options_fn), Loader=yaml.SafeLoader)['dispatcher'] + + @pytest.fixture def dispatcher_test_conf_with_matrix_options(dispatcher_test_conf_with_matrix_options_fn): yield yaml.load(open(dispatcher_test_conf_with_matrix_options_fn), Loader=yaml.SafeLoader)['dispatcher'] + @pytest.fixture def dispatcher_test_conf_with_gallery_no_resolver(dispatcher_test_conf_with_gallery_no_resolver_fn): yield yaml.load(open(dispatcher_test_conf_with_gallery_no_resolver_fn), Loader=yaml.SafeLoader)['dispatcher'] diff --git a/tests/conftest.py b/tests/conftest.py index 1f6659bdc..f026f5bc2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,9 +18,11 @@ gunicorn_dispatcher_long_living_fixture_with_matrix_options, dispatcher_test_conf, dispatcher_test_conf_with_gallery, + dispatcher_test_conf_with_vo_options, dispatcher_test_conf_with_gallery_no_resolver, dispatcher_test_conf_empty_sentry_fn, dispatcher_test_conf_with_gallery_fn, + dispatcher_test_conf_with_vo_options_fn, dispatcher_test_conf_with_gallery_no_resolver_fn, dispatcher_live_fixture_with_external_products_url, dispatcher_live_fixture_with_default_route_products_url, diff --git a/tests/test_server_basic.py b/tests/test_server_basic.py index 25af7d2fb..0258bd9fc 100644 --- a/tests/test_server_basic.py +++ b/tests/test_server_basic.py @@ -2692,6 +2692,53 @@ def test_source_resolver(dispatcher_live_fixture_with_gallery, dispatcher_test_c .format(urllib.parse.quote(source_to_resolve.strip())) +@pytest.mark.test_drupal +@pytest.mark.parametrize("source_to_resolve", ['Mrk 421', 'Mrk_421', 'GX 1+4', 'fake object', None]) +def test_source_resolver_invalid_local_resolver(dispatcher_live_fixture_with_gallery_invalid_local_resolver, dispatcher_test_conf_with_gallery_invalid_local_resolver, source_to_resolve): + server = dispatcher_live_fixture_with_gallery_invalid_local_resolver + + logger.info("constructed server: %s", server) + + # let's generate a valid token + token_payload = { + **default_token_payload, + "roles": "general, gallery contributor", + } + encoded_token = jwt.encode(token_payload, secret_key, algorithm='HS256') + + params = {'name': source_to_resolve, + 'token': encoded_token} + + c = requests.get(os.path.join(server, "resolve_name"), + params={**params} + ) + + assert c.status_code == 200 + resolved_obj = c.json() + print('Resolved object returned: ', resolved_obj) + + if source_to_resolve is None: + assert resolved_obj == {} + elif source_to_resolve == 'fake object': + assert 'name' in resolved_obj + assert 'message' in resolved_obj + + # the name resolver replaces automatically underscores with spaces in the returned name + assert resolved_obj['name'] == source_to_resolve + assert resolved_obj['message'] == f'{source_to_resolve} could not be resolved' + else: + assert 'name' in resolved_obj + assert 'DEC' in resolved_obj + assert 'RA' in resolved_obj + assert 'entity_portal_link' in resolved_obj + assert 'object_ids' in resolved_obj + assert 'object_type' in resolved_obj + + assert resolved_obj['name'] == source_to_resolve.replace('_', ' ') + assert resolved_obj['entity_portal_link'] == dispatcher_test_conf_with_gallery["product_gallery_options"]["entities_portal_url"]\ + .format(urllib.parse.quote(source_to_resolve.strip())) + + @pytest.mark.test_drupal @pytest.mark.parametrize("type_group", ['instruments', 'Instruments', 'products', 'sources', 'aaaaaa', '', None]) @pytest.mark.parametrize("parent", ['isgri', 'production', 'all', 'aaaaaa', '', None]) From bfcd77d0c20bf63df4f46b02710b62c4dfc17c2a Mon Sep 17 00:00:00 2001 From: burnout87 Date: Wed, 2 Oct 2024 17:31:06 +0200 Subject: [PATCH 05/13] extended tests --- cdci_data_analysis/pytest_fixtures.py | 43 ++++++++++++++++++++++++++- tests/conftest.py | 3 ++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/cdci_data_analysis/pytest_fixtures.py b/cdci_data_analysis/pytest_fixtures.py index c3c4000c4..71cb21463 100644 --- a/cdci_data_analysis/pytest_fixtures.py +++ b/cdci_data_analysis/pytest_fixtures.py @@ -1,6 +1,7 @@ # this could be a separate package or/and a pytest plugin from json import JSONDecodeError +import responses import sentry_sdk import yaml @@ -604,7 +605,28 @@ def dispatcher_test_conf_with_gallery_fn(dispatcher_test_conf_fn): '\n product_gallery_url: "http://cdciweb02.astro.unige.ch/mmoda/galleryd"' f'\n product_gallery_secret_key: "{os.getenv("DISPATCHER_PRODUCT_GALLERY_SECRET_KEY", "secret_key")}"' '\n product_gallery_timezone: "Europe/Zurich"' - '\n name_resolver_url: "https://resolver-prod.obsuks1.unige.ch/api/v1.1/byname/{}"' + '\n local_name_resolver_url: "https://resolver-prod.obsuks1.unige.ch/api/v1.1/byname/{}"' + '\n external_name_resolver_url: "http://cdsweb.u-strasbg.fr/cgi-bin/nph-sesame/-oxp/NSV?{}"' + '\n entities_portal_url: "http://cdsportal.u-strasbg.fr/?target={}"' + '\n converttime_revnum_service_url: "https://www.astro.unige.ch/mmoda/dispatch-data/gw/timesystem/api/v1.0/converttime/UTC/{}/REVNUM"') + + yield fn + + +@pytest.fixture +def dispatcher_test_conf_with_gallery_invalid_local_resolver_fn(dispatcher_test_conf_fn): + fn = "test-dispatcher-conf-with-gallery.yaml" + + with open(fn, "w") as f: + with open(dispatcher_test_conf_fn) as f_default: + f.write(f_default.read()) + + f.write('\n product_gallery_options:' + '\n product_gallery_url: "http://cdciweb02.astro.unige.ch/mmoda/galleryd"' + f'\n product_gallery_secret_key: "{os.getenv("DISPATCHER_PRODUCT_GALLERY_SECRET_KEY", "secret_key")}"' + '\n product_gallery_timezone: "Europe/Zurich"' + '\n local_name_resolver_url: "http://invalid_url/"' + '\n external_name_resolver_url: "http://cdsweb.u-strasbg.fr/cgi-bin/nph-sesame/-oxp/NSV?{}"' '\n entities_portal_url: "http://cdsportal.u-strasbg.fr/?target={}"' '\n converttime_revnum_service_url: "https://www.astro.unige.ch/mmoda/dispatch-data/gw/timesystem/api/v1.0/converttime/UTC/{}/REVNUM"') @@ -729,6 +751,11 @@ def dispatcher_test_conf_with_gallery(dispatcher_test_conf_with_gallery_fn): yield yaml.load(open(dispatcher_test_conf_with_gallery_fn), Loader=yaml.SafeLoader)['dispatcher'] +@pytest.fixture +def dispatcher_test_conf_with_gallery_invalid_local_resolver(dispatcher_test_conf_with_gallery_invalid_local_resolver_fn): + yield yaml.load(open(dispatcher_test_conf_with_gallery_invalid_local_resolver_fn), Loader=yaml.SafeLoader)['dispatcher'] + + @pytest.fixture def dispatcher_test_conf_with_vo_options(dispatcher_test_conf_with_vo_options_fn): yield yaml.load(open(dispatcher_test_conf_with_vo_options_fn), Loader=yaml.SafeLoader)['dispatcher'] @@ -1147,6 +1174,20 @@ def dispatcher_live_fixture_with_gallery_no_resolver(pytestconfig, dispatcher_te os.kill(pid, signal.SIGINT) +@pytest.fixture +def dispatcher_live_fixture_with_gallery_invalid_local_resolver(pytestconfig, dispatcher_test_conf_with_gallery_invalid_local_resolver_fn, + dispatcher_debug): + dispatcher_state = start_dispatcher(pytestconfig.rootdir, dispatcher_test_conf_with_gallery_invalid_local_resolver_fn) + + service = dispatcher_state['url'] + pid = dispatcher_state['pid'] + + yield service + + kill_child_processes(pid, signal.SIGINT) + os.kill(pid, signal.SIGINT) + + @pytest.fixture def dispatcher_live_fixture_no_products_url(pytestconfig, dispatcher_test_conf_no_products_url_fn, dispatcher_debug): dispatcher_state = start_dispatcher(pytestconfig.rootdir, dispatcher_test_conf_no_products_url_fn) diff --git a/tests/conftest.py b/tests/conftest.py index f026f5bc2..25e5194bc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,6 +11,7 @@ dispatcher_live_fixture_no_debug_mode, dispatcher_live_fixture_with_gallery, dispatcher_live_fixture_with_gallery_no_resolver, + dispatcher_live_fixture_with_gallery_invalid_local_resolver, dispatcher_long_living_fixture, gunicorn_dispatcher_long_living_fixture, dispatcher_long_living_fixture_with_matrix_options, @@ -20,8 +21,10 @@ dispatcher_test_conf_with_gallery, dispatcher_test_conf_with_vo_options, dispatcher_test_conf_with_gallery_no_resolver, + dispatcher_test_conf_with_gallery_invalid_local_resolver, dispatcher_test_conf_empty_sentry_fn, dispatcher_test_conf_with_gallery_fn, + dispatcher_test_conf_with_gallery_invalid_local_resolver_fn, dispatcher_test_conf_with_vo_options_fn, dispatcher_test_conf_with_gallery_no_resolver_fn, dispatcher_live_fixture_with_external_products_url, From 505ebaf3b924b2bfa067c1aace6de54852efd273 Mon Sep 17 00:00:00 2001 From: burnout87 Date: Wed, 2 Oct 2024 17:34:46 +0200 Subject: [PATCH 06/13] adapted test --- cdci_data_analysis/config_dir/conf_env.yml.example | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cdci_data_analysis/config_dir/conf_env.yml.example b/cdci_data_analysis/config_dir/conf_env.yml.example index 2dd658681..f5aec142e 100644 --- a/cdci_data_analysis/config_dir/conf_env.yml.example +++ b/cdci_data_analysis/config_dir/conf_env.yml.example @@ -115,8 +115,10 @@ dispatcher: product_gallery_secret_key: PRODUCT_GALLERY_SECRET_KEY # timezone used within the drupal configuration, these two values have to be always aligned product_gallery_timezone: PRODUCT_GALLERY_SECRET_KEY - # url of the name resolver - name_resolver_url: NAME_RESOLVER_URL + # url of the local name resolver + local_name_resolver_url: NAME_RESOLVER_URL + # url of the external name resolver + external_name_resolver_url: NAME_RESOLVER_URL # url of the online catalog for astrophysical entities entities_portal_url: ENTITIES_PORTAL_URL # url for the conversion of a given time, in UTC format, to the correspondent REVNUM From 985ef36aed9d267725d7910ee4f660ab69bcb9b2 Mon Sep 17 00:00:00 2001 From: burnout87 Date: Wed, 2 Oct 2024 18:33:49 +0200 Subject: [PATCH 07/13] removed unused import --- cdci_data_analysis/pytest_fixtures.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cdci_data_analysis/pytest_fixtures.py b/cdci_data_analysis/pytest_fixtures.py index 71cb21463..74b91092f 100644 --- a/cdci_data_analysis/pytest_fixtures.py +++ b/cdci_data_analysis/pytest_fixtures.py @@ -1,7 +1,6 @@ # this could be a separate package or/and a pytest plugin from json import JSONDecodeError -import responses import sentry_sdk import yaml From 3daa5fe29a773734e043118ee4e37ff3e18550c9 Mon Sep 17 00:00:00 2001 From: burnout87 Date: Thu, 3 Oct 2024 13:36:15 +0200 Subject: [PATCH 08/13] adapted test --- tests/test_configurer.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_configurer.py b/tests/test_configurer.py index c5f81c1c7..7f77dc3a4 100644 --- a/tests/test_configurer.py +++ b/tests/test_configurer.py @@ -98,7 +98,9 @@ def test_confenv_legacy_plugin_keys(caplog): def test_config_no_resolver_urls(dispatcher_test_conf_with_gallery_no_resolver_fn): conf = ConfigEnv.from_conf_file(dispatcher_test_conf_with_gallery_no_resolver_fn) - assert hasattr(conf, 'name_resolver_url') - assert conf.name_resolver_url is not None + assert hasattr(conf, 'local_name_resolver_url') + assert conf.local_name_resolver_url is not None assert hasattr(conf, 'entities_portal_url') assert conf.entities_portal_url is not None + assert hasattr(conf, 'external_name_resolver_url') + assert conf.external_name_resolver_url is not None From a4c66c734c6d11fd42070c854a64411feff3aff3 Mon Sep 17 00:00:00 2001 From: burnout87 Date: Thu, 3 Oct 2024 15:31:15 +0200 Subject: [PATCH 09/13] using Simbad to get type and ids when using external resolver --- cdci_data_analysis/analysis/drupal_helper.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/cdci_data_analysis/analysis/drupal_helper.py b/cdci_data_analysis/analysis/drupal_helper.py index 3a3beb9a2..f9c4495a1 100644 --- a/cdci_data_analysis/analysis/drupal_helper.py +++ b/cdci_data_analysis/analysis/drupal_helper.py @@ -21,6 +21,7 @@ from enum import Enum, auto from astropy.coordinates import SkyCoord, Angle from astropy import units as u +from astroquery.simbad import Simbad import xml.etree.ElementTree as ET from cdci_data_analysis.analysis import tokenHelper @@ -1556,6 +1557,24 @@ def resolve_name(local_name_resolver_url: str, external_name_resolver_url: str, resolved_obj['RA'] = float(ra_tag.text) resolved_obj['DEC'] = float(dec_tag.text) resolved_obj['entity_portal_link'] = entities_portal_url.format(quoted_name) + + try: + Simbad.add_votable_fields("otype") + result_table = Simbad.query_object(quoted_name) + object_type = str(result_table[0]['OTYPE']).strip() + resolved_obj['object_type'] = object_type + except Exception as e: + logger.warning(f"An exception occurred while using Simbad to query the object \"{name}\" " + f"while using the external resolver:\n{str(e)}") + resolved_obj['object_type'] = None + try: + object_ids_table = Simbad.query_objectids(name) + source_ids_list = object_ids_table['ID'].tolist() + resolved_obj['object_ids'] = source_ids_list + except Exception as e: + logger.warning(f"An exception occurred while using Simbad to query the object ids for the object \"{name}\" " + f"while using the external resolver:\n{str(e)}") + resolved_obj['object_ids'] = None else: warning_msg = ("There seems to be some problem in completing the request for the resolution of the object" f" \"{name}\" using the external resolver.") From d48bccec3eb3627389709d3aa56771068524141d Mon Sep 17 00:00:00 2001 From: burnout87 Date: Thu, 3 Oct 2024 15:46:16 +0200 Subject: [PATCH 10/13] catching exception requesting external resolver --- cdci_data_analysis/analysis/drupal_helper.py | 127 ++++++++++--------- 1 file changed, 69 insertions(+), 58 deletions(-) diff --git a/cdci_data_analysis/analysis/drupal_helper.py b/cdci_data_analysis/analysis/drupal_helper.py index f9c4495a1..4b4e46e0e 100644 --- a/cdci_data_analysis/analysis/drupal_helper.py +++ b/cdci_data_analysis/analysis/drupal_helper.py @@ -1539,70 +1539,81 @@ def resolve_name(local_name_resolver_url: str, external_name_resolver_url: str, sentry.capture_message(f'An exception occurred while trying to resolve the object "{name}" using the local resolver. ' f'URL: {local_name_resolver_url_formatted} ' f"Exception details: {str(e)}") - res = requests.get(external_name_resolver_url.format(quoted_name)) - if res.status_code == 200: - root = ET.fromstring(res.text) - resolved_obj['name'] = name.replace('_', ' ') - resolver_tag = root.find('.//Resolver') - if resolver_tag is not None: - ra_tag = resolver_tag.find('.//jradeg') - dec_tag = resolver_tag.find('.//jdedeg') - if ra_tag is None or dec_tag is None: - info_tag = root.find('.//INFO') + external_name_resolver_url_formatted = external_name_resolver_url.format(quoted_name) + try: + res = requests.get(external_name_resolver_url_formatted) + if res.status_code == 200: + root = ET.fromstring(res.text) + resolved_obj['name'] = name.replace('_', ' ') + resolver_tag = root.find('.//Resolver') + if resolver_tag is not None: + ra_tag = resolver_tag.find('.//jradeg') + dec_tag = resolver_tag.find('.//jdedeg') + if ra_tag is None or dec_tag is None: + info_tag = root.find('.//INFO') + resolved_obj['message'] = f'{name} could not be resolved' + if info_tag is not None: + message_info = info_tag.text + resolved_obj['message'] += f': {message_info}' + else: + resolved_obj['RA'] = float(ra_tag.text) + resolved_obj['DEC'] = float(dec_tag.text) + resolved_obj['entity_portal_link'] = entities_portal_url.format(quoted_name) + + try: + Simbad.add_votable_fields("otype") + result_table = Simbad.query_object(quoted_name) + object_type = str(result_table[0]['OTYPE']).strip() + resolved_obj['object_type'] = object_type + except Exception as e: + logger.warning(f"An exception occurred while using Simbad to query the object \"{name}\" " + f"while using the external resolver:\n{str(e)}") + resolved_obj['object_type'] = None + try: + object_ids_table = Simbad.query_objectids(name) + source_ids_list = object_ids_table['ID'].tolist() + resolved_obj['object_ids'] = source_ids_list + except Exception as e: + logger.warning(f"An exception occurred while using Simbad to query the object ids for the object \"{name}\" " + f"while using the external resolver:\n{str(e)}") + resolved_obj['object_ids'] = None + else: + warning_msg = ("There seems to be some problem in completing the request for the resolution of the object" + f" \"{name}\" using the external resolver.") resolved_obj['message'] = f'{name} could not be resolved' + info_tag = root.find('.//INFO') if info_tag is not None: - message_info = info_tag.text - resolved_obj['message'] += f': {message_info}' - else: - resolved_obj['RA'] = float(ra_tag.text) - resolved_obj['DEC'] = float(dec_tag.text) - resolved_obj['entity_portal_link'] = entities_portal_url.format(quoted_name) - - try: - Simbad.add_votable_fields("otype") - result_table = Simbad.query_object(quoted_name) - object_type = str(result_table[0]['OTYPE']).strip() - resolved_obj['object_type'] = object_type - except Exception as e: - logger.warning(f"An exception occurred while using Simbad to query the object \"{name}\" " - f"while using the external resolver:\n{str(e)}") - resolved_obj['object_type'] = None - try: - object_ids_table = Simbad.query_objectids(name) - source_ids_list = object_ids_table['ID'].tolist() - resolved_obj['object_ids'] = source_ids_list - except Exception as e: - logger.warning(f"An exception occurred while using Simbad to query the object ids for the object \"{name}\" " - f"while using the external resolver:\n{str(e)}") - resolved_obj['object_ids'] = None + warning_msg += (f"The request lead to the error {info_tag.text}, " + "this might be due to an error in the name of the object that ha been provided.") + resolved_obj['message'] += f': {info_tag.text}' + logger.warning(warning_msg) + if sentry_dsn is not None: + sentry.capture_message(f'Failed to resolve object "{name}" using the external resolver. ' + f'URL: {external_name_resolver_url_formatted} ' + f'Status Code: {res.status_code} ' + f'Response: {res.text}' + f"Info returned from the resolver: {resolved_obj['message']}") else: - warning_msg = ("There seems to be some problem in completing the request for the resolution of the object" - f" \"{name}\" using the external resolver.") - resolved_obj['message'] = f'{name} could not be resolved' - info_tag = root.find('.//INFO') - if info_tag is not None: - warning_msg += (f"The request lead to the error {info_tag.text}, " - "this might be due to an error in the name of the object that ha been provided.") - resolved_obj['message'] += f': {info_tag.text}' - logger.warning(warning_msg) + logger.warning("There seems to be some problem in completing the request for the resolution of the object" + f" \"{name}\" using the external resolver.\n" + f"The request lead to the error {res.text}, " + "this might be due to an error in the url or the service " + "requested is currently not available. The object could not be resolved.") if sentry_dsn is not None: - sentry.capture_message(f'Failed to resolve object "{name}" using the remote resolver. ' - f'URL: {local_name_resolver_url.format(quoted_name)} ' + sentry.capture_message(f'Failed to resolve object "{name}" using the external resolver. ' + f'URL: {external_name_resolver_url_formatted} ' f'Status Code: {res.status_code} ' - f'Response: {res.text}' - f"Info returned from the resolver: {resolved_obj['message']}") - else: - logger.warning("There seems to be some problem in completing the request for the resolution of the object" - f" \"{name}\" using the external resolver.\n" - f"The request lead to the error {res.text}, " - "this might be due to an error in the url or the service " - "requested is currently not available. The object could not be resolved.") + f'Response: {res.text}') + resolved_obj['message'] = f'{name} could not be resolved: {res.text}' + except (ConnectionError, + requests.exceptions.ConnectionError, + requests.exceptions.Timeout) as e: + logger.warning(f'An exception occurred while trying to resolve the object "{name}" using the local resolver. ' + f'using the url: {external_name_resolver_url_formatted}. Exception details: {str(e)}') if sentry_dsn is not None: - sentry.capture_message(f'Failed to resolve object "{name}" using the remote resolver. ' - f'URL: {local_name_resolver_url.format(quoted_name)} ' - f'Status Code: {res.status_code} ' - f'Response: {res.text}') - resolved_obj['message'] = f'{name} could not be resolved: {res.text}' + sentry.capture_message(f'An exception occurred while trying to resolve the object "{name}" using the external resolver. ' + f'URL: {external_name_resolver_url_formatted} ' + f"Exception details: {str(e)}") return resolved_obj From 6420a17b8d7c8d704e94f1f8b3367473d6718469 Mon Sep 17 00:00:00 2001 From: burnout87 Date: Thu, 3 Oct 2024 16:03:03 +0200 Subject: [PATCH 11/13] extended test --- tests/test_server_basic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_server_basic.py b/tests/test_server_basic.py index 0258bd9fc..6612e4b27 100644 --- a/tests/test_server_basic.py +++ b/tests/test_server_basic.py @@ -2678,7 +2678,7 @@ def test_source_resolver(dispatcher_live_fixture_with_gallery, dispatcher_test_c # the name resolver replaces automatically underscores with spaces in the returned name assert resolved_obj['name'] == source_to_resolve - assert resolved_obj['message'] == f'{source_to_resolve} could not be resolved' + assert resolved_obj['message'].startswith(f'{source_to_resolve} could not be resolved') else: assert 'name' in resolved_obj assert 'DEC' in resolved_obj @@ -2725,7 +2725,7 @@ def test_source_resolver_invalid_local_resolver(dispatcher_live_fixture_with_gal # the name resolver replaces automatically underscores with spaces in the returned name assert resolved_obj['name'] == source_to_resolve - assert resolved_obj['message'] == f'{source_to_resolve} could not be resolved' + assert resolved_obj['message'].startswith(f'{source_to_resolve} could not be resolved') else: assert 'name' in resolved_obj assert 'DEC' in resolved_obj @@ -2735,7 +2735,7 @@ def test_source_resolver_invalid_local_resolver(dispatcher_live_fixture_with_gal assert 'object_type' in resolved_obj assert resolved_obj['name'] == source_to_resolve.replace('_', ' ') - assert resolved_obj['entity_portal_link'] == dispatcher_test_conf_with_gallery["product_gallery_options"]["entities_portal_url"]\ + assert resolved_obj['entity_portal_link'] == dispatcher_test_conf_with_gallery_invalid_local_resolver["product_gallery_options"]["entities_portal_url"]\ .format(urllib.parse.quote(source_to_resolve.strip())) From 47eb149c4771eeb71a152e0a6f422def0d49844b Mon Sep 17 00:00:00 2001 From: burnout87 Date: Thu, 3 Oct 2024 16:42:38 +0200 Subject: [PATCH 12/13] astroquery version freeze --- requirements.txt | 1 + setup.py | 1 + 2 files changed, 2 insertions(+) diff --git a/requirements.txt b/requirements.txt index 7e13b3e3d..b931918b9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,7 @@ pyyaml simplejson flask==2.0.3 astropy>=5.0.1 +astroquery>=0.4.2 pylogstash_context>=0.1.19 gunicorn decorator diff --git a/setup.py b/setup.py index de2adfba3..698052c02 100644 --- a/setup.py +++ b/setup.py @@ -26,6 +26,7 @@ "simplejson", "flask==2.0.3", "astropy>=2.0.3", + "astroquery>=0.4.2", "gunicorn", "decorator", "python-logstash", From 7a39559b8ebff0c24fb1fc6a71f9b1cc1aa3f84b Mon Sep 17 00:00:00 2001 From: burnout87 Date: Thu, 3 Oct 2024 17:32:10 +0200 Subject: [PATCH 13/13] removed astroquery dependencies --- requirements.txt | 1 - setup.py | 1 - 2 files changed, 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index b931918b9..7e13b3e3d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,6 @@ pyyaml simplejson flask==2.0.3 astropy>=5.0.1 -astroquery>=0.4.2 pylogstash_context>=0.1.19 gunicorn decorator diff --git a/setup.py b/setup.py index 698052c02..de2adfba3 100644 --- a/setup.py +++ b/setup.py @@ -26,7 +26,6 @@ "simplejson", "flask==2.0.3", "astropy>=2.0.3", - "astroquery>=0.4.2", "gunicorn", "decorator", "python-logstash",