From c1df70815c592326e97f60ceefbce8be67086bfd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 29 Sep 2026 14:47:15 +0000 Subject: [PATCH] Replace application prints and bare excepts with logging. Application print calls now go through logging.getLogger(__name__). Bare excepts in the dump download and Geonames update catch Exception, so SystemExit and KeyboardInterrupt are not swallowed. Co-authored-by: Joseph Rhoads --- rorapi/common/create_update.py | 10 ++- rorapi/common/csv_bulk.py | 14 ++-- rorapi/common/csv_create.py | 12 ++-- rorapi/common/csv_update.py | 84 +++++++++++++----------- rorapi/common/csv_utils.py | 44 +++++++------ rorapi/common/record_utils.py | 8 ++- rorapi/common/views.py | 12 ++-- rorapi/management/commands/getrordump.py | 4 +- rorapi/management/commands/setup.py | 4 +- rorapi/settings.py | 20 +++++- 10 files changed, 129 insertions(+), 83 deletions(-) diff --git a/rorapi/common/create_update.py b/rorapi/common/create_update.py index f965fba..05bc337 100644 --- a/rorapi/common/create_update.py +++ b/rorapi/common/create_update.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import copy import functools import json @@ -60,10 +64,10 @@ def update_locations(locations): for location in locations: if 'geonames_id' in location: try: - print(location['geonames_id']) + logger.info(location['geonames_id']) updated_location = ua.new_geonames_v2(str(location['geonames_id'])) updated_locations.append(updated_location['location']) - except: + except Exception: error = "Error retrieving Geonames data for ID {}. Please check that this is a valid Geonames ID".format(location['geonames_id']) return error, updated_locations @@ -90,7 +94,7 @@ def new_record_from_json(json_input, version): new_record['locations'] = updated_locations new_record = add_created_last_mod(new_record) new_ror_id = check_ror_id() - print("new ror id: " + new_ror_id) + logger.info("new ror id: " + new_ror_id) new_record['id'] = new_ror_id error, valid_data = validate_record(sort_list_fields(new_record), get_v2_schema()) return error, valid_data diff --git a/rorapi/common/csv_bulk.py b/rorapi/common/csv_bulk.py index c6dbe8f..d5ee30b 100644 --- a/rorapi/common/csv_bulk.py +++ b/rorapi/common/csv_bulk.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import csv import json import io @@ -43,7 +47,7 @@ def save_report_file(report, report_fields, csv_file, dir_name, validate_only): f.write(chunk) def process_csv(csv_file, version, validate_only): - print("Processing CSV") + logger.info("Processing CSV") dir_name = datetime.now().strftime("%Y-%m-%d_%H_%M_%S") + "-ror-records" success_msg = None error = None @@ -53,15 +57,15 @@ def process_csv(csv_file, version, validate_only): updated_count = 0 new_count = 0 read_file = csv_file.read().decode('utf-8') - print(read_file) + logger.info(read_file) reader = csv.DictReader(io.StringIO(read_file)) row_num = 2 for row in reader: html_url = None ror_id = None updated = False - print("Row data") - print(row) + logger.info("Row data") + logger.info(row) if row['html_url']: html_url = row['html_url'] if row['id']: @@ -80,7 +84,7 @@ def process_csv(csv_file, version, validate_only): ror_id = v2_record['id'] serializer = OrganizationSerializerV2(v2_record) json_obj = json.loads(JSONRenderer().render(serializer.data)) - print(json_obj) + logger.info(json_obj) if not validate_only: #create file file = save_record_file(ror_id, updated, json_obj, dir_name) diff --git a/rorapi/common/csv_create.py b/rorapi/common/csv_create.py index 10ff03b..154b90b 100644 --- a/rorapi/common/csv_create.py +++ b/rorapi/common/csv_create.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import copy from rorapi.common.record_utils import * from rorapi.common.csv_utils import * @@ -74,8 +78,8 @@ def new_record_from_csv(csv_data, version): "lang": lang_code } temp_names.append(name_obj) - print("temp names 1:") - print(temp_names) + logger.info("temp names 1:") + logger.info(temp_names) name_vals = [n['value'] for n in temp_names] dup_names = [] for n in name_vals: @@ -99,8 +103,8 @@ def new_record_from_csv(csv_data, version): if name_obj not in temp_names: temp_names = [t for t in temp_names if t not in name_lang_dups] temp_names.append(name_obj) - print("temp names 2:") - print(temp_names) + logger.info("temp names 2:") + logger.info(temp_names) v2_data['names'] = temp_names #status diff --git a/rorapi/common/csv_update.py b/rorapi/common/csv_update.py index 2299e2a..16cb78d 100644 --- a/rorapi/common/csv_update.py +++ b/rorapi/common/csv_update.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import copy from rorapi.common.record_utils import * from rorapi.v2.record_constants import * @@ -12,29 +16,29 @@ def update_record_from_csv(csv_data, version): errors = [] updated_record = None - print("updating record from csv") + logger.info("updating record from csv") existing_org_errors, existing_org = retrieve_organization(csv_data['id']) - print(existing_org) + logger.info(existing_org) if existing_org is None: errors.append("No existing record found for ROR ID '{}'".format(csv_data['id'])) else: row_validation_errors = validate_csv_row_update_syntax(csv_data) if row_validation_errors: errors.extend(row_validation_errors) - print("row validation errors:") - print(errors) + logger.info("row validation errors:") + logger.info(errors) else: serializer = OrganizationSerializerV2(existing_org) existing_record = serializer.data - print(existing_record) + logger.info(existing_record) update_data = {} #domains if csv_data['domains']: actions_values = get_actions_values(csv_data['domains']) temp_domains = copy.deepcopy(existing_record['domains']) - print("initial temp domains:") - print(temp_domains) + logger.info("initial temp domains:") + logger.info(temp_domains) if UPDATE_ACTIONS['DELETE'] in actions_values: delete_values = actions_values[UPDATE_ACTIONS['DELETE']] if delete_values is None: @@ -45,23 +49,23 @@ def update_record_from_csv(csv_data, version): errors.append("Attempting to delete domain(s) that don't exist: {}".format(d)) temp_domains = [d for d in temp_domains if d not in delete_values] - print("temp domains delete") - print(temp_domains) + logger.info("temp domains delete") + logger.info(temp_domains) if UPDATE_ACTIONS['ADD'] in actions_values: add_values = actions_values[UPDATE_ACTIONS['ADD']] for a in add_values: if a in temp_domains: errors.append("Attempting to add domain(s) that already exist: {}".format(a)) - print(add_values) + logger.info(add_values) temp_domains.extend(add_values) - print("temp domains add") - print(temp_domains) + logger.info("temp domains add") + logger.info(temp_domains) if UPDATE_ACTIONS['REPLACE'] in actions_values: temp_domains = actions_values[UPDATE_ACTIONS['REPLACE']] - print("temp domains replace") - print(temp_domains) - print("final temp domains:") - print(temp_domains) + logger.info("temp domains replace") + logger.info(temp_domains) + logger.info("final temp domains:") + logger.info(temp_domains) update_data['domains'] = temp_domains #established @@ -180,18 +184,18 @@ def update_record_from_csv(csv_data, version): "value": r } temp_links.append(link_obj) - print("final temp links:") - print(temp_links) + logger.info("final temp links:") + logger.info(temp_links) update_data['links'] = temp_links #locations if csv_data['locations.geonames_id']: actions_values = get_actions_values(csv_data['locations.geonames_id']) temp_locations = copy.deepcopy(existing_record['locations']) - print("initial temp locations:") - print(temp_locations) + logger.info("initial temp locations:") + logger.info(temp_locations) existing_geonames_ids = [tl['geonames_id'] for tl in temp_locations] - print(existing_geonames_ids) + logger.info(existing_geonames_ids) if UPDATE_ACTIONS['DELETE'] in actions_values: delete_values = [int(d) for d in actions_values[UPDATE_ACTIONS['DELETE']]] for d in delete_values: @@ -219,8 +223,8 @@ def update_record_from_csv(csv_data, version): "geonames_details": {} } temp_locations.append(location_obj) - print("final temp locations:") - print(temp_locations) + logger.info("final temp locations:") + logger.info(temp_locations) update_data['locations'] = temp_locations #names @@ -228,20 +232,20 @@ def update_record_from_csv(csv_data, version): for k,v in V2_NAME_TYPES.items(): if csv_data['names.types.' + v]: updated_name_types.append(v) - print("updated name types") - print(updated_name_types) + logger.info("updated name types") + logger.info(updated_name_types) if updated_name_types: temp_names = copy.deepcopy(existing_record['names']) for t in updated_name_types: - print("updating name type " + t) + logger.info("updating name type " + t) if csv_data['names.types.' + t]: actions_values = get_actions_values(csv_data['names.types.' + t]) for k, v in actions_values.items(): if v: vals_obj_list = [] for val in v: - print("val is") - print(val) + logger.info("val is") + logger.info(val) vals_obj = { "value": None, "lang": None @@ -263,12 +267,12 @@ def update_record_from_csv(csv_data, version): if vals_obj["value"]: vals_obj_list.append(vals_obj) actions_values[k] = vals_obj_list - print("updated actions values") - print(actions_values) + logger.info("updated actions values") + logger.info(actions_values) if UPDATE_ACTIONS['DELETE'] in actions_values: - print("delete in actions") + logger.info("delete in actions") delete_values = actions_values[UPDATE_ACTIONS['DELETE']] - print(delete_values) + logger.info(delete_values) if delete_values is None: temp_names = [tn for tn in temp_names if t not in tn['types']] else: @@ -299,8 +303,8 @@ def update_record_from_csv(csv_data, version): else: name_vals_match = [tn for tn in temp_names if (tn['value'] == a['value'] and tn['lang'] == a['lang'])] if name_vals_match: - print("name vals match") - print(name_vals_match) + logger.info("name vals match") + logger.info(name_vals_match) for nvm in name_vals_match: # if value and lang exist but not type, add type only if len(nvm['types']) > 0: @@ -346,8 +350,8 @@ def update_record_from_csv(csv_data, version): } temp_names.append(name_obj) - print("final temp names:") - print(temp_names) + logger.info("final temp names:") + logger.info(temp_names) update_data['names'] = temp_names #status @@ -362,8 +366,8 @@ def update_record_from_csv(csv_data, version): if csv_data['types']: actions_values = get_actions_values(csv_data['types']) temp_types = copy.deepcopy(existing_record['types']) - print("initial temp types:") - print(temp_types) + logger.info("initial temp types:") + logger.info(temp_types) if UPDATE_ACTIONS['DELETE'] in actions_values: delete_values = [av.lower() for av in actions_values[UPDATE_ACTIONS['DELETE']]] for d in delete_values: @@ -380,8 +384,8 @@ def update_record_from_csv(csv_data, version): temp_types.extend(add_values) if UPDATE_ACTIONS['REPLACE'] in actions_values: temp_types = [av.lower() for av in actions_values[UPDATE_ACTIONS['REPLACE']]] - print("final temp types:") - print(temp_types) + logger.info("final temp types:") + logger.info(temp_types) update_data['types'] = temp_types if not errors: diff --git a/rorapi/common/csv_utils.py b/rorapi/common/csv_utils.py index 8c4d706..d0e8e1f 100644 --- a/rorapi/common/csv_utils.py +++ b/rorapi/common/csv_utils.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import csv import io import re @@ -43,27 +47,27 @@ def get_actions_values(csv_field): - print("getting actions values:") + logger.info("getting actions values:") actions_values = {} if csv_field.lower() == UPDATE_ACTIONS["DELETE"]: actions_values[UPDATE_ACTIONS["DELETE"]] = None elif UPDATE_DELIMITER in csv_field: for ua in list(UPDATE_ACTIONS.values()): - print(ua) + logger.info(ua) if ua + UPDATE_DELIMITER in csv_field: - print("doing regex:") + logger.info("doing regex:") regex = r"(" + re.escape( ua + UPDATE_DELIMITER) + r")(.*?)(?=$|(add|delete|replace)==)" result = re.search(regex, csv_field) - print(result[0]) + logger.info(result[0]) temp_val = result[0].replace(ua + UPDATE_DELIMITER, '') - print("temp val:") - print(temp_val) + logger.info("temp val:") + logger.info(temp_val) actions_values[ua] = [v.strip() for v in temp_val.split(';') if v] else: actions_values[UPDATE_ACTIONS["REPLACE"]] = [v.strip() for v in csv_field.split(';') if v] - print(actions_values) + logger.info(actions_values) return actions_values def validate_csv(csv_file): @@ -80,28 +84,28 @@ def validate_csv(csv_file): for field in CSV_REQUIRED_FIELDS_ACTIONS.keys(): if field not in csv_fields: missing_fields.append(field) - print(missing_fields) + logger.info(missing_fields) if missing_fields: errors.append(f'CSV file is missing columns: {", ".join(missing_fields)}') else: errors.append("CSV file contains no data rows") except IOError as e: errors.append(f"Error parsing CSV file: {e}") - print(errors) + logger.info(errors) return errors def validate_csv_row_update_syntax(csv_data): - print("validating row") + logger.info("validating row") errors = [] for k, v in csv_data.items(): if UPDATE_DELIMITER in v: - print("field:") - print(k) - print("value:") - print(v) + logger.info("field:") + logger.info(k) + logger.info("value:") + logger.info(v) actions_values = get_actions_values(v) - print("actions values:") - print(actions_values) + logger.info("actions values:") + logger.info(actions_values) update_actions = list(actions_values.keys()) if not update_actions: errors.append("Update delimiter '{}' found in '{}' field but no valid update action found in value {}".format(UPDATE_DELIMITER, k, v)) @@ -111,10 +115,10 @@ def validate_csv_row_update_syntax(csv_data): if not (UPDATE_ACTIONS['ADD'] and UPDATE_ACTIONS['DELETE']) in update_actions: errors.append("Invalid combination of update actions '{}' found in '{}' field.".format(", ".join(update_actions), k)) disallowed_actions = [ua for ua in update_actions if ua not in CSV_REQUIRED_FIELDS_ACTIONS[k]] - print("allowed actions:") - print(CSV_REQUIRED_FIELDS_ACTIONS[k]) - print("disallowed actions:") - print(disallowed_actions) + logger.info("allowed actions:") + logger.info(CSV_REQUIRED_FIELDS_ACTIONS[k]) + logger.info("disallowed actions:") + logger.info(disallowed_actions) if disallowed_actions: errors.append("Invalid update action(s) '{}' found in {} field. Allowed actions for this field are '{}'".format(", ".join(disallowed_actions), k, ", ".join(CSV_REQUIRED_FIELDS_ACTIONS[k]))) if v.strip() == UPDATE_ACTIONS['DELETE'].lower() and k in NO_DELETE_FIELDS: diff --git a/rorapi/common/record_utils.py b/rorapi/common/record_utils.py index 2332785..a8e2fee 100644 --- a/rorapi/common/record_utils.py +++ b/rorapi/common/record_utils.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import jsonschema import requests from iso639 import Lang @@ -24,8 +28,8 @@ def get_file_from_url(url): def validate_record(data, schema): try: - print("validating data:") - print(data) + logger.info("validating data:") + logger.info(data) jsonschema.validate(data, schema) except jsonschema.ValidationError as e: return "Validation error: " + e.message, None diff --git a/rorapi/common/views.py b/rorapi/common/views.py index 68aa99d..0bbc8c9 100644 --- a/rorapi/common/views.py +++ b/rorapi/common/views.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import csv from rest_framework import viewsets, routers, status from rest_framework.response import Response @@ -160,7 +164,7 @@ class OrganizationViewSet(viewsets.ViewSet): def list(self, request, version=REST_FRAMEWORK["DEFAULT_VERSION"]): params = request.GET.dict() if "query.name" in params or "query.names" in params: - print("redirecting") + logger.info("redirecting") param_name = "query.name" if "query.name" in params else "query.names" params["query"] = params[param_name] del params[param_name] @@ -276,7 +280,7 @@ class GenerateId(APIView): def get(self, request, version=REST_FRAMEWORK["DEFAULT_VERSION"]): id = check_ror_id() - print("Generated ID: {}".format(id)) + logger.info("Generated ID: {}".format(id)) return Response({"id": id}) class IndexData(APIView): @@ -319,7 +323,7 @@ def post(self, request, version=REST_FRAMEWORK["DEFAULT_VERSION"]): errors = Errors(["File upload required. 'file' field is missing."]) else: mime_type = magic.from_buffer(file_object.read(2048)) - print(mime_type) + logger.info(mime_type) if "ASCII text" in mime_type or "UTF-8 text" in mime_type or "UTF-8 Unicode text" in mime_type or "CSV text" in mime_type: file_object.seek(0) csv_validation_errors = validate_csv(file_object) @@ -338,7 +342,7 @@ def post(self, request, version=REST_FRAMEWORK["DEFAULT_VERSION"]): else: errors = Errors(["Could not process request. No data included in request."]) if errors is not None: - print(errors.__dict__) + logger.info(errors.__dict__) return Response( ErrorsSerializer(errors).data, status=status.HTTP_400_BAD_REQUEST ) diff --git a/rorapi/management/commands/getrordump.py b/rorapi/management/commands/getrordump.py index cb75d74..2e2d570 100644 --- a/rorapi/management/commands/getrordump.py +++ b/rorapi/management/commands/getrordump.py @@ -27,7 +27,7 @@ def get_ror_dump_sha(filename, use_test_data, github_headers): if filename in file['name']: sha = file['sha'] return sha - except: + except Exception: return None def get_ror_dump_zip(self, filename, use_test_data, github_headers): @@ -52,7 +52,7 @@ def get_ror_dump_zip(self, filename, use_test_data, github_headers): if dir_names: raise SystemExit(f"Dump zip has extra directory and cannot be indexed") return zip_file.name - except: + except Exception: raise SystemExit(f"Something went wrong saving zip file") class Command(BaseCommand): diff --git a/rorapi/management/commands/setup.py b/rorapi/management/commands/setup.py index fc9f8bf..018a933 100644 --- a/rorapi/management/commands/setup.py +++ b/rorapi/management/commands/setup.py @@ -111,9 +111,9 @@ def handle(self, *args, **options): filename = options['filename'] use_test_data = options['testdata'] if use_test_data: - print("Using ror-data-test repo") + logger.info("Using ror-data-test repo") else: - print("Using ror-data repo") + logger.info("Using ror-data repo") try: sha = get_ror_dump_sha(filename, use_test_data) diff --git a/rorapi/settings.py b/rorapi/settings.py index dea38ed..8931bbb 100644 --- a/rorapi/settings.py +++ b/rorapi/settings.py @@ -10,6 +10,7 @@ https://docs.djangoproject.com/en/5.2/ref/settings/ """ +import logging import os import sys import json @@ -203,7 +204,8 @@ #DATA['CLIENT'] = localboto3.client('s3') #DATA['OBJECT'] = DATA['CLIENT'].list_objects_v2(Bucket = DATA['DATA_STORE']) else: - print("Please set the DATA_STORE environment variable or run this codebase through docker compose") + logging.getLogger(__name__).warning( + "Please set the DATA_STORE environment variable or run this codebase through docker compose") DATA['DIR'] = os.path.join(BASE_DIR, 'rorapi', 'data') ROR_API = {'PAGE_SIZE': 20, 'ID_PREFIX': 'https://ror.org/'} @@ -220,3 +222,19 @@ AWS_ACCESS_KEY_ID = os.environ.get('AWS_ACCESS_KEY_ID') AWS_SECRET_ACCESS_KEY = os.environ.get('AWS_SECRET_ACCESS_KEY') AWS_SES_REGION_NAME = os.environ.get('AWS_REGION', 'eu-west-1') + +LOGGING = { + 'version': 1, + 'disable_existing_loggers': False, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + }, + }, + 'loggers': { + 'rorapi': { + 'handlers': ['console'], + 'level': 'INFO', + }, + }, +}