From e68f7a131a449ac58405bab82cefa62392769c84 Mon Sep 17 00:00:00 2001 From: Douglas Cerna Date: Tue, 24 Oct 2023 08:21:58 -0600 Subject: [PATCH] Update flake8 configuration * Remove language version settings in pre-commit config * Remove flake8-import-order tox setting * Update tox configuration * Add flake8-bugbear * Add flake8-comprehensions --- .pre-commit-config.yaml | 5 ++-- .../clientScripts/archivematica_clamscan.py | 11 ++++--- .../check_for_access_directory.py | 2 +- .../check_for_service_directory.py | 4 +-- .../lib/clientScripts/create_transfer_mets.py | 4 +-- .../lib/clientScripts/email_fail_report.py | 6 ++-- .../lib/clientScripts/extract_contents.py | 2 +- .../extract_maildir_attachments.py | 16 ++++++---- src/MCPClient/lib/clientScripts/fits.py | 2 +- .../lib/clientScripts/is_maildir_aip.py | 2 +- .../lib/clientScripts/json_metadata_to_csv.py | 2 +- ..._normalization_move_access_files_to_dip.py | 2 +- ...ual_normalization_remove_mn_directories.py | 2 +- .../lib/clientScripts/normalize_report.py | 2 +- .../restructure_dip_for_content_dm_upload.py | 4 +-- .../lib/clientScripts/save_dublin_core.py | 4 ++- .../set_maildir_file_grp_use_and_file_ids.py | 4 +-- .../trim_create_rights_entries.py | 4 +-- .../trim_restructure_for_compliance.py | 2 +- .../clientScripts/trim_verify_checksums.py | 6 ++-- .../lib/clientScripts/trim_verify_manifest.py | 8 ++--- .../lib/clientScripts/upload_archivesspace.py | 2 +- .../lib/clientScripts/verify_checksum.py | 7 ++--- .../clientScripts/verify_sip_compliance.py | 2 +- src/MCPClient/lib/fork_runner.py | 4 ++- ...test_archivematicaCreateMETSMetadataXML.py | 4 ++- .../tests/test_assign_uuids_to_directories.py | 4 +-- src/MCPClient/tests/test_create_aip_mets.py | 2 +- .../tests/test_load_premis_events_from_xml.py | 2 +- src/MCPClient/tests/test_reingest_mets.py | 7 +++-- src/MCPClient/tests/test_verify_checksum.py | 6 ++-- src/MCPServer/lib/server/jobs/decisions.py | 2 +- src/MCPServer/lib/server/packages.py | 4 +-- src/MCPServer/lib/server/rpc_server.py | 2 +- src/MCPServer/lib/server/watch_dirs.py | 1 + src/MCPServer/tests/test_gearman.py | 2 +- src/MCPServer/tests/test_package.py | 24 +++++++-------- .../lib/archivematicaFunctions.py | 2 +- src/archivematicaCommon/lib/bindpid.py | 12 ++++---- src/archivematicaCommon/lib/dicts.py | 6 ++-- .../lib/elasticSearchFunctions.py | 17 +++++------ src/archivematicaCommon/lib/email_settings.py | 30 +++++++++---------- .../lib/executeOrRunSubProcess.py | 25 +++++++++++++--- src/archivematicaCommon/lib/fileOperations.py | 4 ++- .../tests/test_elasticsearch_functions.py | 3 +- .../src/components/accounts/views.py | 2 +- .../src/components/administration/views.py | 6 ++-- .../src/components/advanced_search.py | 18 ++++++----- .../src/components/api/validators.py | 2 +- .../tests/test_archival_storage.py | 2 +- .../src/components/filesystem_ajax/helpers.py | 6 ++-- src/dashboard/src/components/helpers.py | 4 +-- .../src/components/ingest/pair_matcher.py | 14 ++++++--- src/dashboard/src/components/ingest/views.py | 16 +++++----- src/dashboard/src/components/rights/views.py | 16 +++++----- .../src/components/transfer/views.py | 2 +- .../migrations/0035_python3_compatibility.py | 2 +- src/dashboard/src/fpr/views.py | 2 +- .../commands/rebuild_transfer_backlog.py | 12 ++++---- .../commands/resolve_pending_jobs.py | 2 +- src/dashboard/src/main/models.py | 9 ++---- .../src/main/templatetags/breadcrumb.py | 4 +-- src/dashboard/tests/test_api.py | 6 ++-- src/dashboard/tests/test_filesystem_ajax.py | 2 +- src/dashboard/tests/test_transfer.py | 8 ++--- tox.ini | 17 +++++++++-- 66 files changed, 230 insertions(+), 190 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9c2bbb3c74..7113687191 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,9 +29,10 @@ repos: hooks: - id: black args: [--safe, --quiet] - language_version: python3 - repo: https://github.com/pycqa/flake8 rev: "6.1.0" hooks: - id: flake8 - language_version: python3 + additional_dependencies: + - flake8-bugbear==23.9.16 + - flake8-comprehensions==3.14.0 diff --git a/src/MCPClient/lib/clientScripts/archivematica_clamscan.py b/src/MCPClient/lib/clientScripts/archivematica_clamscan.py index 0802f32046..aceaa88c32 100755 --- a/src/MCPClient/lib/clientScripts/archivematica_clamscan.py +++ b/src/MCPClient/lib/clientScripts/archivematica_clamscan.py @@ -122,10 +122,9 @@ def scan(self, path): state, details = result[result_key] except Exception as err: passed = ClamdScanner.clamd_exception_handler(err) - finally: - if state == "OK": - passed = True - return passed, state, details + if state == "OK": + passed = True + return passed, state, details @staticmethod def clamd_exception_handler(err): @@ -297,7 +296,7 @@ def get_size(file_uuid, path): # Our fallback. try: return os.path.getsize(path) - except: + except Exception: return None @@ -349,7 +348,7 @@ def scan_file(event_queue, file_uuid, path, date, task_uuid): else: passed, state, details = None, None, None - except: + except Exception: logger.error("Unexpected error scanning file %s", path, exc_info=True) return 1 else: diff --git a/src/MCPClient/lib/clientScripts/check_for_access_directory.py b/src/MCPClient/lib/clientScripts/check_for_access_directory.py index 4ccdf81054..d7c833cead 100755 --- a/src/MCPClient/lib/clientScripts/check_for_access_directory.py +++ b/src/MCPClient/lib/clientScripts/check_for_access_directory.py @@ -166,7 +166,7 @@ def call(jobs): os.mkdir(DIPDirectory) if not os.path.isdir(os.path.join(DIPDirectory, "objects")): os.mkdir(os.path.join(DIPDirectory, "objects")) - except: + except Exception: job.pyprint("error creating DIP directory") exitCode = main( diff --git a/src/MCPClient/lib/clientScripts/check_for_service_directory.py b/src/MCPClient/lib/clientScripts/check_for_service_directory.py index 76eee31f10..a88dfe712d 100755 --- a/src/MCPClient/lib/clientScripts/check_for_service_directory.py +++ b/src/MCPClient/lib/clientScripts/check_for_service_directory.py @@ -32,7 +32,7 @@ def something(job, SIPDirectory, serviceDirectory, objectsDirectory, SIPUUID, da exitCode = 0 job.pyprint(SIPDirectory) # For every file, & directory Try to find the matching file & directory in the objects directory - for path, dirs, files in os.walk(serviceDirectory): + for path, _, files in os.walk(serviceDirectory): for file in files: servicePreExtension = "_me" originalPreExtension = "_m" @@ -79,7 +79,7 @@ def regular(SIPDirectory, objectsDirectory, SIPUUID, date): if not searchForRegularExpressions: return - for path, dirs, files in os.walk(objectsDirectory): + for path, _, files in os.walk(objectsDirectory): for file in files: m = re.search(r"_me\.[a-zA-Z0-9]{2,4}$", file) if m is not None: diff --git a/src/MCPClient/lib/clientScripts/create_transfer_mets.py b/src/MCPClient/lib/clientScripts/create_transfer_mets.py index cc76636148..61be662130 100755 --- a/src/MCPClient/lib/clientScripts/create_transfer_mets.py +++ b/src/MCPClient/lib/clientScripts/create_transfer_mets.py @@ -256,11 +256,11 @@ def load_rights_data_from_db(self): ) for rights in transfer_rights: - for path, fsentry in self.file_index.items(): + for _, fsentry in self.file_index.items(): premis_rights = rights_to_premis(rights, fsentry.file_uuid) fsentry.add_premis_rights(premis_rights) - for path, fsentry in self.file_index.items(): + for _, fsentry in self.file_index.items(): file_rights = self.rights_queryset.filter( metadataappliestoidentifier=fsentry.file_uuid, metadataappliestotype_id=self.FILE_RIGHTS_LOOKUP_UUID, diff --git a/src/MCPClient/lib/clientScripts/email_fail_report.py b/src/MCPClient/lib/clientScripts/email_fail_report.py index e95a7dfb89..8d8872b371 100755 --- a/src/MCPClient/lib/clientScripts/email_fail_report.py +++ b/src/MCPClient/lib/clientScripts/email_fail_report.py @@ -57,7 +57,7 @@ def send_email(subject, to, content): recipient_list=to, html_message=content, ) - except: + except Exception: logger.exception("Report email was not delivered") raise else: @@ -171,7 +171,7 @@ def get_content_for(unit_type, unit_name, unit_uuid, html=True): else: root.append(t1) etree.SubElement(root, "p") - except: + except Exception: pass html2code = get_unit_job_log_html(unit_uuid) @@ -255,7 +255,7 @@ def call(jobs): # Generate report in plain text and store it in the database with transaction.atomic(): - for arg in reports_to_store: + for args in reports_to_store: content = get_content_for( args.unit_type, args.unit_name, args.unit_uuid, html=False ) diff --git a/src/MCPClient/lib/clientScripts/extract_contents.py b/src/MCPClient/lib/clientScripts/extract_contents.py index ba67acc8c3..b32b108e21 100755 --- a/src/MCPClient/lib/clientScripts/extract_contents.py +++ b/src/MCPClient/lib/clientScripts/extract_contents.py @@ -131,7 +131,7 @@ def main(job, transfer_uuid, sip_directory, date, task_uuid, delete=False): try: format_id = FileFormatVersion.objects.get(file_uuid=file_.uuid) # Can't do anything if the file wasn't identified in the previous step - except: + except Exception: job.pyprint( "Not extracting contents from", os.path.basename(file_.currentlocation.decode()), diff --git a/src/MCPClient/lib/clientScripts/extract_maildir_attachments.py b/src/MCPClient/lib/clientScripts/extract_maildir_attachments.py index 13d31f8389..d699e95cc4 100755 --- a/src/MCPClient/lib/clientScripts/extract_maildir_attachments.py +++ b/src/MCPClient/lib/clientScripts/extract_maildir_attachments.py @@ -46,7 +46,7 @@ def __init__(self): def writeFile(filePath, fileContents): try: os.makedirs(os.path.dirname(filePath)) - except: + except Exception: pass FILE = open(filePath, "w") FILE.writelines(fileContents) @@ -59,9 +59,11 @@ def addFile( transferUUID, date, eventDetail="", - fileUUID=uuid.uuid4().__str__(), + fileUUID=None, ): - taskUUID = uuid.uuid4().__str__() + if fileUUID is None: + fileUUID = str(uuid.uuid4()) + taskUUID = str(uuid.uuid4()) filePathRelativeToSIP = filePath.replace(transferPath, "%transferDirectory%", 1) addFileToTransfer( filePathRelativeToSIP, @@ -72,7 +74,7 @@ def addFile( sourceType="unpacking", eventDetail=eventDetail, ) - updateSizeAndChecksum(fileUUID, filePath, date, uuid.uuid4.__str__()) + updateSizeAndChecksum(fileUUID, filePath, date, str(uuid.uuid4())) def getFileUUIDofSourceFile(transferUUID, sourceFilePath): @@ -93,8 +95,10 @@ def addKeyFileToNormalizeMaildirOffOf( transferUUID, date, eventDetail="", - fileUUID=uuid.uuid4().__str__(), + fileUUID=None, ): + if fileUUID is None: + fileUUID = str(uuid.uuid4()) basename = os.path.basename(mirrorDir) dirname = os.path.dirname(mirrorDir) outFile = os.path.join(dirname, basename + ".archivematicaMaildir") @@ -248,7 +252,7 @@ def handle_job(job): mirrorDir = os.path.join(transferDir, "objects/attachments", maildirsub) try: os.makedirs(mirrorDir) - except: + except Exception: pass eventDetail = "added for normalization purposes" fileUUID = uuid.uuid4().__str__() diff --git a/src/MCPClient/lib/clientScripts/fits.py b/src/MCPClient/lib/clientScripts/fits.py index fcf9f18844..2a93bd1723 100755 --- a/src/MCPClient/lib/clientScripts/fits.py +++ b/src/MCPClient/lib/clientScripts/fits.py @@ -88,7 +88,7 @@ def main(target, xml_file, date, event_uuid, file_uuid, file_grpuse): try: tree = etree.parse(temp_file) - except: + except Exception: logger.exception("Failed to read Fits's XML.") return 2 diff --git a/src/MCPClient/lib/clientScripts/is_maildir_aip.py b/src/MCPClient/lib/clientScripts/is_maildir_aip.py index da1232e35f..c94613a3ea 100755 --- a/src/MCPClient/lib/clientScripts/is_maildir_aip.py +++ b/src/MCPClient/lib/clientScripts/is_maildir_aip.py @@ -31,7 +31,7 @@ def isMaildir(path): for maildirsub2 in os.listdir(maildir): maildirsub = os.path.join(maildir, maildirsub2) mailbox.Maildir(maildirsub, None) - except: + except Exception: return False return True diff --git a/src/MCPClient/lib/clientScripts/json_metadata_to_csv.py b/src/MCPClient/lib/clientScripts/json_metadata_to_csv.py index 6d25b4b28d..2ff69a1731 100755 --- a/src/MCPClient/lib/clientScripts/json_metadata_to_csv.py +++ b/src/MCPClient/lib/clientScripts/json_metadata_to_csv.py @@ -30,7 +30,7 @@ def fetch_keys(objects): # Column order is important so the output is consistent. # "filename" and "parts" must be column 0. # (They are mutually exclusive.) - keys = sorted(list(keys)) + keys = sorted(keys) if "filename" in keys: keys.remove("filename") keys.insert(0, "filename") diff --git a/src/MCPClient/lib/clientScripts/manual_normalization_move_access_files_to_dip.py b/src/MCPClient/lib/clientScripts/manual_normalization_move_access_files_to_dip.py index 059e00be29..99a355fb56 100755 --- a/src/MCPClient/lib/clientScripts/manual_normalization_move_access_files_to_dip.py +++ b/src/MCPClient/lib/clientScripts/manual_normalization_move_access_files_to_dip.py @@ -141,7 +141,7 @@ def main(job): try: if not os.path.isdir(dstDir): os.makedirs(dstDir) - except: + except Exception: pass # Rename the file or directory src to dst. If dst is a directory, OSError will be raised. On Unix, if dst exists and is a file, it will be replaced silently if the user has permission. The operation may fail on some Unix flavors if src and dst are on different filesystems. diff --git a/src/MCPClient/lib/clientScripts/manual_normalization_remove_mn_directories.py b/src/MCPClient/lib/clientScripts/manual_normalization_remove_mn_directories.py index e8643ede44..f58bd7182a 100755 --- a/src/MCPClient/lib/clientScripts/manual_normalization_remove_mn_directories.py +++ b/src/MCPClient/lib/clientScripts/manual_normalization_remove_mn_directories.py @@ -32,7 +32,7 @@ def recursivelyRemoveEmptyDirectories(job, dir): error_count = 0 - for root, dirs, files in os.walk(dir, topdown=False): + for root, dirs, _ in os.walk(dir, topdown=False): for directory in dirs: try: os.rmdir(os.path.join(root, directory)) diff --git a/src/MCPClient/lib/clientScripts/normalize_report.py b/src/MCPClient/lib/clientScripts/normalize_report.py index 3eac0d1da1..b12b02f098 100755 --- a/src/MCPClient/lib/clientScripts/normalize_report.py +++ b/src/MCPClient/lib/clientScripts/normalize_report.py @@ -184,7 +184,7 @@ def report(uuid): recipient_list=recipient_list, html_message=html_message, ) - except: + except Exception: logger.exception("Report email was not delivered") return 1 else: diff --git a/src/MCPClient/lib/clientScripts/restructure_dip_for_content_dm_upload.py b/src/MCPClient/lib/clientScripts/restructure_dip_for_content_dm_upload.py index 838ac8b122..70cbac9263 100755 --- a/src/MCPClient/lib/clientScripts/restructure_dip_for_content_dm_upload.py +++ b/src/MCPClient/lib/clientScripts/restructure_dip_for_content_dm_upload.py @@ -82,9 +82,9 @@ def getItemCountType(structMap): """ divs_with_dmdsecs = structMap.findall(".//mets:div[@DMDID]", namespaces=ns.NSMAP) # If any are TYPE Directory, then it is compound - if any([e.get("TYPE") == "Directory" for e in divs_with_dmdsecs]): + if any(e.get("TYPE") == "Directory" for e in divs_with_dmdsecs): # If all are TYPE Directory then it is bulk - if all([e.get("TYPE") == "Directory" for e in divs_with_dmdsecs]): + if all(e.get("TYPE") == "Directory" for e in divs_with_dmdsecs): return "compound-dirs" else: return "compound-files" diff --git a/src/MCPClient/lib/clientScripts/save_dublin_core.py b/src/MCPClient/lib/clientScripts/save_dublin_core.py index 8b7ca5e930..450ca8b081 100755 --- a/src/MCPClient/lib/clientScripts/save_dublin_core.py +++ b/src/MCPClient/lib/clientScripts/save_dublin_core.py @@ -34,7 +34,9 @@ def main(job, transfer_uuid, target_path): jsonified = {} try: dc = models.DublinCore.objects.get(metadataappliestoidentifier=transfer_uuid) - except: # There may not be any DC metadata for this transfer, and that's fine + except ( + Exception + ): # There may not be any DC metadata for this transfer, and that's fine job.pyprint("No DC metadata found; skipping", file=sys.stderr) return 0 for field in FIELDS: diff --git a/src/MCPClient/lib/clientScripts/set_maildir_file_grp_use_and_file_ids.py b/src/MCPClient/lib/clientScripts/set_maildir_file_grp_use_and_file_ids.py index 40edb45819..c909c26e59 100755 --- a/src/MCPClient/lib/clientScripts/set_maildir_file_grp_use_and_file_ids.py +++ b/src/MCPClient/lib/clientScripts/set_maildir_file_grp_use_and_file_ids.py @@ -64,7 +64,7 @@ def set_maildir_files(sip_uuid, sip_path): maildir_path, sip_uuid, ) - for root, dirs, files in os.walk(maildir_path): + for root, _, files in os.walk(maildir_path): for item in files: file_relative_path = os.path.join(root, item).replace( sip_path, "%SIPDirectory%", 1 @@ -84,7 +84,7 @@ def set_archivematica_maildir_files(sip_uuid, sip_path): attachments_path, sip_uuid, ) - for root, dirs, files in os.walk(attachments_path): + for root, _, files in os.walk(attachments_path): for item in files: if not item.endswith(".archivematicaMaildir"): continue diff --git a/src/MCPClient/lib/clientScripts/trim_create_rights_entries.py b/src/MCPClient/lib/clientScripts/trim_create_rights_entries.py index 8dcb8dba62..e42223083f 100755 --- a/src/MCPClient/lib/clientScripts/trim_create_rights_entries.py +++ b/src/MCPClient/lib/clientScripts/trim_create_rights_entries.py @@ -109,7 +109,7 @@ def call(jobs): try: tree = etree.parse(xmlFilePath) root = tree.getroot() - except: + except Exception: job.pyprint( "Error parsing: ", xmlFilePath.replace(transferPath, "%transferDirectory%", 1), @@ -122,7 +122,7 @@ def call(jobs): "Container/RetentionSchedule" ).text DateClosed = root.find("Container/DateClosed").text - except: + except Exception: job.pyprint( "Error retrieving values from: ", xmlFilePath.replace(transferPath, "%transferDirectory%", 1), diff --git a/src/MCPClient/lib/clientScripts/trim_restructure_for_compliance.py b/src/MCPClient/lib/clientScripts/trim_restructure_for_compliance.py index ba4628772a..0b8f7fbf27 100755 --- a/src/MCPClient/lib/clientScripts/trim_restructure_for_compliance.py +++ b/src/MCPClient/lib/clientScripts/trim_restructure_for_compliance.py @@ -107,7 +107,7 @@ def restructureTRIMForComplianceFileUUIDsAssigned( files = fileOperations.getFileUUIDLike( dst, unitPath, unitIdentifier, unitIdentifierType, unitPathReplaceWith ) - for key, value in files.items(): + for value in files.values(): fileUUID = value fileOperations.updateFileGrpUse(fileUUID, "TRIM metadata") diff --git a/src/MCPClient/lib/clientScripts/trim_verify_checksums.py b/src/MCPClient/lib/clientScripts/trim_verify_checksums.py index a642a01bb0..71bf441bbf 100755 --- a/src/MCPClient/lib/clientScripts/trim_verify_checksums.py +++ b/src/MCPClient/lib/clientScripts/trim_verify_checksums.py @@ -67,7 +67,7 @@ def call(jobs): root = tree.getroot() xmlMD5 = root.find("Document/MD5").text - except: + except Exception: job.pyprint("Error parsing: ", xmlFilePath, file=sys.stderr) exitCode += 1 continue @@ -88,14 +88,14 @@ def call(jobs): "transfer", "%transferDirectory%", ) - for path, fileUUID in fileID.items(): + for fileUUID in fileID.values(): eventDetail = 'program="python"; module="hashlib.md5()"' eventOutcome = "Pass" eventOutcomeDetailNote = "{} {}".format( xmlFile.__str__(), "verified", ) - eventIdentifierUUID = uuid.uuid4().__str__() + eventIdentifierUUID = str(uuid.uuid4()) databaseFunctions.insertIntoEvents( fileUUID=fileUUID, diff --git a/src/MCPClient/lib/clientScripts/trim_verify_manifest.py b/src/MCPClient/lib/clientScripts/trim_verify_manifest.py index 9cb45b01b2..1853fdd674 100755 --- a/src/MCPClient/lib/clientScripts/trim_verify_manifest.py +++ b/src/MCPClient/lib/clientScripts/trim_verify_manifest.py @@ -118,11 +118,11 @@ def call(jobs): file=sys.stderr, ) exitCode += 1 - for paths, fileUUID in fileID.items(): + for fileUUID in fileID.values(): eventDetail = 'program="archivematica"; module="trimVerifyManifest"' eventOutcome = "Pass" eventOutcomeDetailNote = "Verified file exists" - eventIdentifierUUID = uuid.uuid4().__str__() + eventIdentifierUUID = str(uuid.uuid4()) databaseFunctions.insertIntoEvents( fileUUID=fileUUID, eventIdentifierUUID=eventIdentifierUUID, @@ -158,11 +158,11 @@ def call(jobs): file=sys.stderr, ) exitCode += 1 - for paths, fileUUID in fileID.items(): + for fileUUID in fileID.values(): eventDetail = 'program="archivematica"; module="trimVerifyManifest"' eventOutcome = "Pass" eventOutcomeDetailNote = "Verified file exists, but with implicit extension case" - eventIdentifierUUID = uuid.uuid4().__str__() + eventIdentifierUUID = str(uuid.uuid4()) databaseFunctions.insertIntoEvents( fileUUID=fileUUID, eventIdentifierUUID=eventIdentifierUUID, diff --git a/src/MCPClient/lib/clientScripts/upload_archivesspace.py b/src/MCPClient/lib/clientScripts/upload_archivesspace.py index 5054c5ef72..5723e9ad56 100755 --- a/src/MCPClient/lib/clientScripts/upload_archivesspace.py +++ b/src/MCPClient/lib/clientScripts/upload_archivesspace.py @@ -26,7 +26,7 @@ def recursive_file_gen(mydir): - for root, dirs, files in os.walk(mydir): + for root, _, files in os.walk(mydir): for file in files: yield os.path.join(root, file) diff --git a/src/MCPClient/lib/clientScripts/verify_checksum.py b/src/MCPClient/lib/clientScripts/verify_checksum.py index 46ecced726..20b9f562a3 100755 --- a/src/MCPClient/lib/clientScripts/verify_checksum.py +++ b/src/MCPClient/lib/clientScripts/verify_checksum.py @@ -191,12 +191,9 @@ def get_ext(path): @staticmethod def _count_lines(path): """Count the number of lines in a checksum file.""" - count = 0 with open(path) as hashfile: - for count, _ in enumerate(hashfile): - pass - # Negate zero-based count. - return count + 1 + count = sum(1 for _ in hashfile) + return count @staticmethod def _count_files(path): diff --git a/src/MCPClient/lib/clientScripts/verify_sip_compliance.py b/src/MCPClient/lib/clientScripts/verify_sip_compliance.py index c54aaac10c..b68d15953a 100755 --- a/src/MCPClient/lib/clientScripts/verify_sip_compliance.py +++ b/src/MCPClient/lib/clientScripts/verify_sip_compliance.py @@ -33,7 +33,7 @@ def checkDirectory(job, directory, ret=0): try: - for directory, subDirectories, files in os.walk(directory): + for _, _, files in os.walk(directory): for file in files: os.path.join(directory, file) except Exception as inst: diff --git a/src/MCPClient/lib/fork_runner.py b/src/MCPClient/lib/fork_runner.py index 0d078ab2c9..c9311683e1 100755 --- a/src/MCPClient/lib/fork_runner.py +++ b/src/MCPClient/lib/fork_runner.py @@ -31,11 +31,13 @@ THIS_SCRIPT = "fork_runner.py" -def call(module_name, jobs, task_count=multiprocessing.cpu_count()): +def call(module_name, jobs, task_count=None): """ Split `jobs` into `task_count` groups and fork a subprocess to run `module_name`.call() for each of them. """ + if task_count is None: + task_count = multiprocessing.cpu_count() jobs_by_uuid = {} for job in jobs: jobs_by_uuid[job.UUID] = job diff --git a/src/MCPClient/tests/test_archivematicaCreateMETSMetadataXML.py b/src/MCPClient/tests/test_archivematicaCreateMETSMetadataXML.py index 1a02fa83a5..43275c0912 100644 --- a/src/MCPClient/tests/test_archivematicaCreateMETSMetadataXML.py +++ b/src/MCPClient/tests/test_archivematicaCreateMETSMetadataXML.py @@ -102,7 +102,9 @@ def _make_mock_fsentry(**kwargs): @pytest.fixture def make_mock_mets(mocker, make_mock_fsentry): - def _make_mock_mets(metadata_file_uuids=[]): + def _make_mock_mets(metadata_file_uuids=None): + if metadata_file_uuids is None: + metadata_file_uuids = [] sip = make_mock_fsentry(label="sip", type="Directory") objects = make_mock_fsentry(label="objects", type="Directory") directory = make_mock_fsentry(label="directory", type="Directory") diff --git a/src/MCPClient/tests/test_assign_uuids_to_directories.py b/src/MCPClient/tests/test_assign_uuids_to_directories.py index 500ff985d5..47ae9b7fa7 100755 --- a/src/MCPClient/tests/test_assign_uuids_to_directories.py +++ b/src/MCPClient/tests/test_assign_uuids_to_directories.py @@ -36,11 +36,11 @@ def test_main(mocker, transfer, transfer_dir): include_dirs = True # Verify there are no directories. - models.Directory.objects.count() == 0 + assert models.Directory.objects.count() == 0 result = main(job, str(transfer_dir), transfer.uuid, include_dirs) assert result == 0 # Verify two directories were created. - models.Directory.objects.filter(transfer=transfer).count() == 2 + assert models.Directory.objects.filter(transfer=transfer).count() == 2 diff --git a/src/MCPClient/tests/test_create_aip_mets.py b/src/MCPClient/tests/test_create_aip_mets.py index f454b8a356..cacaff65be 100644 --- a/src/MCPClient/tests/test_create_aip_mets.py +++ b/src/MCPClient/tests/test_create_aip_mets.py @@ -826,7 +826,7 @@ def generate_aip_mets_v2_state(self): ) self.state = create_mets_v2.MetsState() self.state.globalStructMapCounter = random.choice( - [x for x in range(arbitrary_max_structmaps)] + list(range(arbitrary_max_structmaps)) ) self.structmap_div_element = create_mets_v2.createFileSec( job=Job("stub", "stub", []), diff --git a/src/MCPClient/tests/test_load_premis_events_from_xml.py b/src/MCPClient/tests/test_load_premis_events_from_xml.py index 9178cb328f..7589c2156d 100644 --- a/src/MCPClient/tests/test_load_premis_events_from_xml.py +++ b/src/MCPClient/tests/test_load_premis_events_from_xml.py @@ -223,7 +223,7 @@ def test_get_premis_element_children_identifiers(mocker): "params", [ {"original_name": "name", "expected_original_name": "name"}, - {"original_name": tuple(), "expected_original_name": ""}, + {"original_name": (), "expected_original_name": ""}, ], ids=["original_name_as_string", "original_name_as_empty_string"], ) diff --git a/src/MCPClient/tests/test_reingest_mets.py b/src/MCPClient/tests/test_reingest_mets.py index 379ef3ecc8..c836670fce 100644 --- a/src/MCPClient/tests/test_reingest_mets.py +++ b/src/MCPClient/tests/test_reingest_mets.py @@ -1981,8 +1981,11 @@ def test_new_dmdsecs_for_directories(self): dmdsec = ( mets.get_file(label="Landing_zone", type="Directory").dmdsecs[0].serialize() ) - dmdsec.findtext(".//dc:title", namespaces=NSMAP) == "The landing zone" - dmdsec.findtext(".//dc:description", namespaces=NSMAP) == "A zone for landing" + assert dmdsec.findtext(".//dc:title", namespaces=NSMAP) == "The landing zone" + assert ( + dmdsec.findtext(".//dc:description", namespaces=NSMAP) + == "A zone for landing" + ) def test_update_existing(self): """ diff --git a/src/MCPClient/tests/test_verify_checksum.py b/src/MCPClient/tests/test_verify_checksum.py index f39cda9f5f..5afdbe4ffe 100644 --- a/src/MCPClient/tests/test_verify_checksum.py +++ b/src/MCPClient/tests/test_verify_checksum.py @@ -355,7 +355,7 @@ def test_write_premis_event_to_db(self, transfer): idtypes = [] agentnames = [] agenttypes = [] - for agent_count, agent in enumerate(event.agents.all(), 1): + for _agent_count, agent in enumerate(event.agents.all(), 1): idvalues.append(agent.identifiervalue) idtypes.append(agent.identifiertype) agentnames.append(agent.name) @@ -371,9 +371,9 @@ def test_write_premis_event_to_db(self, transfer): ), "agent name values returned don't match" assert set(agenttypes) == set(agent_types), "agent types don't match" assert ( - agent_count == number_of_expected_agents + _agent_count == number_of_expected_agents ), "Number of agents is incorrect: {} expected: {}".format( - agent_count, number_of_expected_agents + _agent_count, number_of_expected_agents ) # Collect the different checksum algorithms written to ensure they # were all written independently in the function. diff --git a/src/MCPServer/lib/server/jobs/decisions.py b/src/MCPServer/lib/server/jobs/decisions.py index da9bdc5d7a..2f6767fa95 100644 --- a/src/MCPServer/lib/server/jobs/decisions.py +++ b/src/MCPServer/lib/server/jobs/decisions.py @@ -133,7 +133,7 @@ def get_preconfigured_choice(self): self.package.current_path, self.link.id ) if desired_choice and self.job_chain.generated_choices: - for key, data in self.job_chain.generated_choices.items(): + for _, data in self.job_chain.generated_choices.items(): if data["uri"] == desired_choice: return data["uri"] diff --git a/src/MCPServer/lib/server/packages.py b/src/MCPServer/lib/server/packages.py index 13224cd4d0..cad8587b7c 100644 --- a/src/MCPServer/lib/server/packages.py +++ b/src/MCPServer/lib/server/packages.py @@ -681,7 +681,7 @@ def files( files_returned_already.add(file_obj_mapped.get("%inputFile%")) yield file_obj_mapped - for basedir, subdirs, files in os.walk(start_path): + for basedir, _, files in os.walk(start_path): for file_name in files: if ( filter_filename_start @@ -712,7 +712,7 @@ def set_variable(self, key, value, chain_link_id): unittype=self.UNIT_VARIABLE_TYPE, unituuid=self.uuid, variable=key, - defaults=dict(variablevalue=value, microservicechainlink=chain_link_id), + defaults={"variablevalue": value, "microservicechainlink": chain_link_id}, ) if created: message = "New UnitVariable %s created for %s: %s (MSCL: %s)" diff --git a/src/MCPServer/lib/server/rpc_server.py b/src/MCPServer/lib/server/rpc_server.py index f9e6fb9bca..5c5cde9204 100644 --- a/src/MCPServer/lib/server/rpc_server.py +++ b/src/MCPServer/lib/server/rpc_server.py @@ -218,7 +218,7 @@ def _job_awaiting_approval_handler(self, worker, job): raise_exc = True """ ret = etree.Element("choicesAvailableForUnits") - for uuid, choice in self.package_queue.jobs_awaiting_decisions().items(): + for choice in self.package_queue.jobs_awaiting_decisions().values(): unit_choices = etree.SubElement(ret, "choicesAvailableForUnit") etree.SubElement(unit_choices, "UUID").text = str(choice.uuid) unit = etree.SubElement(unit_choices, "unit") diff --git a/src/MCPServer/lib/server/watch_dirs.py b/src/MCPServer/lib/server/watch_dirs.py index bb6daa75ff..9ff80586b0 100644 --- a/src/MCPServer/lib/server/watch_dirs.py +++ b/src/MCPServer/lib/server/watch_dirs.py @@ -74,6 +74,7 @@ def watch_directories_inotify( warnings.warn( "inotify may not work as a watched directory method on non-linux systems.", RuntimeWarning, + stacklevel=2, ) inotify = INotify() diff --git a/src/MCPServer/tests/test_gearman.py b/src/MCPServer/tests/test_gearman.py index 91f2b3ceef..26de4fbb4b 100644 --- a/src/MCPServer/tests/test_gearman.py +++ b/src/MCPServer/tests/test_gearman.py @@ -193,7 +193,7 @@ def test_gearman_multiple_batches( backend = GearmanTaskBackend() job_requests = [] - for i in range(3): + for _ in range(3): mock_gearman_job = mocker.Mock() job_request = gearman.job.GearmanJobRequest( mock_gearman_job, background=True, max_attempts=0 diff --git a/src/MCPServer/tests/test_package.py b/src/MCPServer/tests/test_package.py index b0d7fa4116..068db7b078 100644 --- a/src/MCPServer/tests/test_package.py +++ b/src/MCPServer/tests/test_package.py @@ -185,7 +185,7 @@ def test_reload_file_list(tmp_path): # One file will be returned from the database with a UUID, another from # the filesystem without a UUID. - for file_count, file_info in enumerate(transfer.files(None, None, "/objects"), 1): + for _file_count, file_info in enumerate(transfer.files(None, None, "/objects"), 1): assert "%fileUUID%" in file_info assert "%fileGrpUse%" in file_info assert "%relativeLocation%" in file_info @@ -205,7 +205,7 @@ def test_reload_file_list(tmp_path): } models.File.objects.create(**kwargs) assert ( - file_count == 2 + _file_count == 2 ), "Database and file objects were not returned by the generator" assert models.File.objects.filter(transfer_id=str(transfer_uuid)).count() == 2 @@ -215,7 +215,7 @@ def test_reload_file_list(tmp_path): sub_dir.mkdir() new_file = sub_dir / "another_new_file.txt" new_file.touch() - for file_count, file_info in enumerate(transfer.files(None, None, "/objects"), 1): + for _file_count, file_info in enumerate(transfer.files(None, None, "/objects"), 1): if file_info.get("%fileUUID%") != "None": continue file_path = Path( @@ -231,20 +231,20 @@ def test_reload_file_list(tmp_path): } models.File.objects.create(**kwargs) assert ( - file_count == 3 + _file_count == 3 ), "Database and file objects were not returned by the generator" assert models.File.objects.filter(transfer_id=str(transfer_uuid)).count() == 3 # Now the database is updated, we will still have the same file count, but # all objects will be returned from the database and we will have uuids. - for file_count, file_info in enumerate(transfer.files(None, None, "/objects"), 1): + for _file_count, file_info in enumerate(transfer.files(None, None, "/objects"), 1): if file_info.get("%fileUUID%") == "None": - assert ( - False - ), "Non-database entries returned from package.files(): {}".format( - file_info + raise AssertionError( + "Non-database entries returned from package.files(): {}".format( + file_info + ) ) - assert file_count == 3 + assert _file_count == 3 # Finally, let's just see if the scan works for a slightly larger no. # files, i.e. a number with an increment slightly larger than one. @@ -253,11 +253,11 @@ def test_reload_file_list(tmp_path): new_file = objects_path / file_ new_file.touch() new_count = 0 - for file_count, file_info in enumerate(transfer.files(None, None, "/objects"), 1): + for _file_count, file_info in enumerate(transfer.files(None, None, "/objects"), 1): if file_info.get("%fileUUID%") == "None": new_count += 1 assert new_count == 5 - assert file_count == 8 + assert _file_count == 8 # Clean up state and ensure test doesn't interfere with other transfers # expected to be in the database, e.g. in test_queues.py. diff --git a/src/archivematicaCommon/lib/archivematicaFunctions.py b/src/archivematicaCommon/lib/archivematicaFunctions.py index bdf6fadf86..755f61d472 100644 --- a/src/archivematicaCommon/lib/archivematicaFunctions.py +++ b/src/archivematicaCommon/lib/archivematicaFunctions.py @@ -524,4 +524,4 @@ def chunk_iterable(iterable, chunk_size=10, fillvalue=None): [('A', 'B', 'C'), ('D', 'E', 'F'), ('G', 'x', 'x')] """ args = [iter(iterable)] * chunk_size - return zip_longest(fillvalue=fillvalue, *args) + return zip_longest(*args, fillvalue=fillvalue) diff --git a/src/archivematicaCommon/lib/bindpid.py b/src/archivematicaCommon/lib/bindpid.py index ae0ac305c0..01172a5e0c 100755 --- a/src/archivematicaCommon/lib/bindpid.py +++ b/src/archivematicaCommon/lib/bindpid.py @@ -316,12 +316,12 @@ def _render_request_body(argsdict, resolve_url, qualified_resolve_urls): """ return _render_template( argsdict["pid_request_body_template"], - dict( - naming_authority=argsdict["naming_authority"], - pid=argsdict["desired_pid"], - base_resolve_url=resolve_url, - qualified_resolve_urls=qualified_resolve_urls, - ), + { + "naming_authority": argsdict["naming_authority"], + "pid": argsdict["desired_pid"], + "base_resolve_url": resolve_url, + "qualified_resolve_urls": qualified_resolve_urls, + }, ).encode("utf8") diff --git a/src/archivematicaCommon/lib/dicts.py b/src/archivematicaCommon/lib/dicts.py index b800f265e9..e5b4122d4b 100644 --- a/src/archivematicaCommon/lib/dicts.py +++ b/src/archivematicaCommon/lib/dicts.py @@ -90,7 +90,7 @@ def frommodel(type_="file", sip=None, file_=None, expand_path=True): # sip can be a SIP or Transfer try: sip = models.SIP.objects.get(uuid=sip) - except: + except Exception: sip = models.Transfer.objects.get(uuid=sip) shared_path = config["shared_directory"] @@ -100,7 +100,7 @@ def frommodel(type_="file", sip=None, file_=None, expand_path=True): if file_ and not sip: try: sip = file_.sip - except: + except Exception: sip = file_.transfer rd = ReplacementDict() @@ -135,7 +135,7 @@ def frommodel(type_="file", sip=None, file_=None, expand_path=True): rd["%fileUUID%"] = str(file_.uuid) try: base_location = file_.sip.currentpath - except: + except Exception: base_location = file_.transfer.currentlocation if expand_path and sipdir is not None: diff --git a/src/archivematicaCommon/lib/elasticSearchFunctions.py b/src/archivematicaCommon/lib/elasticSearchFunctions.py index 2428d1fe7a..e67b441098 100644 --- a/src/archivematicaCommon/lib/elasticSearchFunctions.py +++ b/src/archivematicaCommon/lib/elasticSearchFunctions.py @@ -102,7 +102,7 @@ class TooManyResultsError(ElasticsearchError): DEPTH_LIMIT = 1000 -def setup(hosts, timeout=DEFAULT_TIMEOUT, enabled=[AIPS_INDEX, TRANSFERS_INDEX]): +def setup(hosts, timeout=DEFAULT_TIMEOUT, enabled=(AIPS_INDEX, TRANSFERS_INDEX)): """Initialize and share the Elasticsearch client. Share it as the attribute _es_client in the current module. An additional @@ -172,7 +172,7 @@ def _wait_for_cluster_yellow_status(client, wait_between_tries=10, max_tries=10) try: health = client.cluster.health() - except: + except Exception: print("ERROR: failed health check.") health["status"] = None @@ -801,9 +801,8 @@ def _index_transfer_files( # The BagIt is created when the package is sent to backlog hence # the locations in the database do not reflect the BagIt paths. # Strip the "data/" part when looking up the file entry. - currentlocation = "%transferDirectory%" + os.path.relpath( - filepath, path - ).lstrip("data/") + stripped_path = re.sub(r"^data/", "", os.path.relpath(filepath, path)) + currentlocation = "%transferDirectory%" + stripped_path try: f = File.objects.get( currentlocation=currentlocation.encode(), transfer_id=uuid @@ -821,7 +820,7 @@ def _index_transfer_files( bulk_extractor_reports = [] # Get file path info - relative_path = filepath.replace(path, transfer_name + "/") + stripped_path = filepath.replace(path, transfer_name + "/") file_extension = os.path.splitext(filepath)[1][1:].lower() filename = os.path.basename(filepath) # Size in megabytes @@ -829,12 +828,12 @@ def _index_transfer_files( create_time = os.stat(filepath).st_ctime if filename not in ignore_files: - printfn(f"Indexing {relative_path} (UUID: {file_uuid})") + printfn(f"Indexing {stripped_path} (UUID: {file_uuid})") # TODO: Index Backlog Location UUID? indexData = { "filename": filename, - "relative_path": relative_path, + "relative_path": stripped_path, "fileuuid": file_uuid, "sipuuid": uuid, "accessionid": accession_id, @@ -856,7 +855,7 @@ def _index_transfer_files( files_indexed = files_indexed + 1 else: - printfn(f"Skipping indexing {relative_path}") + printfn(f"Skipping indexing {stripped_path}") return files_indexed diff --git a/src/archivematicaCommon/lib/email_settings.py b/src/archivematicaCommon/lib/email_settings.py index 5420d0c96e..2e64f489e1 100644 --- a/src/archivematicaCommon/lib/email_settings.py +++ b/src/archivematicaCommon/lib/email_settings.py @@ -59,27 +59,27 @@ def get_settings(config): This should be invoked from a Django settings module and the result merged into the globals() dict. """ - settings = dict( + settings = { # Which backend to use? - EMAIL_BACKEND=config.get("email_backend"), + "EMAIL_BACKEND": config.get("email_backend"), # File Backend # See https://docs.djangoproject.com/en/dev/topics/email/#file-backend - EMAIL_FILE_PATH=config.get("email_file_path"), + "EMAIL_FILE_PATH": config.get("email_file_path"), # SMTP Backend # See https://docs.djangoproject.com/en/dev/topics/email/#smtp-backend - EMAIL_HOST=config.get("email_host"), - EMAIL_HOST_PASSWORD=config.get("email_host_password"), - EMAIL_HOST_USER=config.get("email_host_user"), - EMAIL_PORT=config.get("email_port"), - EMAIL_SSL_CERTFILE=config.get("email_ssl_certfile"), - EMAIL_SSL_KEYFILE=config.get("email_ssl_keyfile"), - EMAIL_USE_SSL=config.get("email_use_ssl"), - EMAIL_USE_TLS=config.get("email_use_tls"), + "EMAIL_HOST": config.get("email_host"), + "EMAIL_HOST_PASSWORD": config.get("email_host_password"), + "EMAIL_HOST_USER": config.get("email_host_user"), + "EMAIL_PORT": config.get("email_port"), + "EMAIL_SSL_CERTFILE": config.get("email_ssl_certfile"), + "EMAIL_SSL_KEYFILE": config.get("email_ssl_keyfile"), + "EMAIL_USE_SSL": config.get("email_use_ssl"), + "EMAIL_USE_TLS": config.get("email_use_tls"), # General settings, not backend-specific - DEFAULT_FROM_EMAIL=config.get("default_from_email"), - EMAIL_SUBJECT_PREFIX=config.get("email_subject_prefix"), - EMAIL_TIMEOUT=config.get("email_timeout", None), - ) + "DEFAULT_FROM_EMAIL": config.get("default_from_email"), + "EMAIL_SUBJECT_PREFIX": config.get("email_subject_prefix"), + "EMAIL_TIMEOUT": config.get("email_timeout", None), + } settings["SERVER_EMAIL"] = config.get("server_email", settings["EMAIL_HOST_USER"]) return settings diff --git a/src/archivematicaCommon/lib/executeOrRunSubProcess.py b/src/archivematicaCommon/lib/executeOrRunSubProcess.py index 4f10685a36..7af0070c73 100644 --- a/src/archivematicaCommon/lib/executeOrRunSubProcess.py +++ b/src/archivematicaCommon/lib/executeOrRunSubProcess.py @@ -24,7 +24,12 @@ def launchSubProcess( - command, stdIn="", printing=True, arguments=[], env_updates={}, capture_output=False + command, + stdIn="", + printing=True, + arguments=None, + env_updates=None, + capture_output=False, ): """ Launches a subprocess using ``command``, where ``command`` is either: @@ -55,6 +60,10 @@ def launchSubProcess( returned IF the subprocess has failed, i.e., returned a non-zero exit code. """ + if arguments is None: + arguments = [] + if env_updates is None: + env_updates = {} stdError = "" stdOut = "" @@ -130,8 +139,12 @@ def launchSubProcess( def createAndRunScript( - text, stdIn="", printing=True, arguments=[], env_updates={}, capture_output=True + text, stdIn="", printing=True, arguments=None, env_updates=None, capture_output=True ): + if arguments is None: + arguments = [] + if env_updates is None: + env_updates = {} # Output the text to a /tmp/ file scriptPath = "/tmp/" + uuid.uuid4().__str__() FILE = os.open(scriptPath, os.O_WRONLY | os.O_CREAT, 0o770) @@ -160,8 +173,8 @@ def executeOrRun( text, stdIn="", printing=True, - arguments=[], - env_updates={}, + arguments=None, + env_updates=None, capture_output=True, ): """ @@ -194,6 +207,10 @@ def executeOrRun( capture_output: Whether or not to capture output for the executed process. Default is `True`. """ + if arguments is None: + arguments = [] + if env_updates is None: + env_updates = {} if type == "command": return launchSubProcess( text, diff --git a/src/archivematicaCommon/lib/fileOperations.py b/src/archivematicaCommon/lib/fileOperations.py index b23f134d46..822c1623a6 100644 --- a/src/archivematicaCommon/lib/fileOperations.py +++ b/src/archivematicaCommon/lib/fileOperations.py @@ -249,7 +249,7 @@ def updateFileLocation( eventType="", eventDateTime="", eventDetail="", - eventIdentifierUUID=uuid.uuid4().__str__(), + eventIdentifierUUID=None, fileUUID="None", sipUUID=None, transferUUID=None, @@ -262,6 +262,8 @@ def updateFileLocation( If the file uuid is not provided, will use the SIP uuid and the old path to find the file uuid. To suppress creation of an event, pass the createEvent keyword argument (for example, if the file moved due to the renaming of a parent directory and not the file itself). """ + if eventIdentifierUUID is None: + eventIdentifierUUID = str(uuid.uuid4()) if not fileUUID or fileUUID == "None": kwargs = {"removedtime__isnull": True, "currentlocation": src.encode()} diff --git a/src/archivematicaCommon/tests/test_elasticsearch_functions.py b/src/archivematicaCommon/tests/test_elasticsearch_functions.py index c2b4d8af8a..037b509d6f 100644 --- a/src/archivematicaCommon/tests/test_elasticsearch_functions.py +++ b/src/archivematicaCommon/tests/test_elasticsearch_functions.py @@ -196,8 +196,7 @@ def _bulk(client, actions, stats_only=False, *args, **kwargs): @mock.patch("elasticSearchFunctions.bulk") def test_index_mets_file_metadata_with_utf8(self, dummy_helpers_bulk): def _bulk(client, actions, stats_only=False, *args, **kwargs): - for action in actions: - pass + pass dummy_helpers_bulk.side_effect = _bulk mets_file_path = os.path.join( diff --git a/src/dashboard/src/components/accounts/views.py b/src/dashboard/src/components/accounts/views.py index b44f74fde6..7a06666715 100644 --- a/src/dashboard/src/components/accounts/views.py +++ b/src/dashboard/src/components/accounts/views.py @@ -191,5 +191,5 @@ def delete(request, id): raise Http404 user.delete() return redirect("accounts:accounts_index") - except: + except Exception: raise Http404 diff --git a/src/dashboard/src/components/administration/views.py b/src/dashboard/src/components/administration/views.py index 7a38b28260..082a6b07d9 100644 --- a/src/dashboard/src/components/administration/views.py +++ b/src/dashboard/src/components/administration/views.py @@ -181,7 +181,7 @@ def storage(request): """ try: response_locations = storage_service.get_location() - except: + except Exception: messages.warning( request, _( @@ -326,7 +326,7 @@ def _get_shared_dirs(calculate_usage=False): ) if calculate_usage: - for id, dir_spec in dirs.items(): + for dir_spec in dirs.values(): dir_spec["used"] = _usage_get_directory_used_bytes(dir_spec["path"]) return dirs @@ -561,7 +561,7 @@ def general(request): ) forms = (general_form, storage_form, checksum_form) - if all([form.is_valid() for form in forms]): + if all(form.is_valid() for form in forms): for item in forms: item.save() messages.info(request, _("Saved.")) diff --git a/src/dashboard/src/components/advanced_search.py b/src/dashboard/src/components/advanced_search.py index 5cd53666c7..f6e9c37425 100644 --- a/src/dashboard/src/components/advanced_search.py +++ b/src/dashboard/src/components/advanced_search.py @@ -46,19 +46,19 @@ def search_parameter_prep(request): fields = [""] else: # Make sure each query has field/ops set - for index, query in enumerate(queries): + for index, _ in enumerate(queries): # A blank query makes ES error if queries[index] == "": queries[index] = "*" try: fields[index] - except: + except Exception: fields.insert(index, "") try: ops[index] - except: + except Exception: ops.insert(index, "and") if ops[index] == "": @@ -66,7 +66,7 @@ def search_parameter_prep(request): try: types[index] - except: + except Exception: types.insert(index, "") # For "other" fields, the actual title of the subfield is located in a @@ -95,18 +95,20 @@ def extract_url_search_params_from_request(request): search_params = request.get_full_path().split("?")[1] end_of_search_params = search_params.index("&page") search_params = search_params[:end_of_search_params] - except: + except Exception: pass return search_params -def assemble_query(queries, ops, fields, types, filters=[]): +def assemble_query(queries, ops, fields, types, filters=None): + if filters is None: + filters = [] must_haves = [] should_haves = [] must_not_haves = [] index = 0 - for query in queries: + for _ in queries: if queries[index] != "": clause = _query_clause(index, queries, ops, fields, types) if clause: @@ -230,5 +232,5 @@ def _query_clause(index, queries, ops, fields, types): def indexed_count(es_client, index, query=None): try: return es_client.count(index=index, body=query)["count"] - except: + except Exception: return 0 diff --git a/src/dashboard/src/components/api/validators.py b/src/dashboard/src/components/api/validators.py index 57f0e0cc7a..bbe9bb47e8 100644 --- a/src/dashboard/src/components/api/validators.py +++ b/src/dashboard/src/components/api/validators.py @@ -155,7 +155,7 @@ def _check_field_pairs(row): :param row: list: metadata fields """ - for i, field in enumerate(row): + for field in row: if field == "Other Identifier Type": if not all( f in row for f in ["Other Identifier", "Other Identifier Type"] diff --git a/src/dashboard/src/components/archival_storage/tests/test_archival_storage.py b/src/dashboard/src/components/archival_storage/tests/test_archival_storage.py index 91964ebf80..819834bc39 100644 --- a/src/dashboard/src/components/archival_storage/tests/test_archival_storage.py +++ b/src/dashboard/src/components/archival_storage/tests/test_archival_storage.py @@ -271,7 +271,7 @@ def test_search_as_csv(mocker, amsetup, admin_client, tmp_path): response.get(CONTENT_DISPOSITION) == 'attachment; filename="test-filename.csv"' ) - streamed_content = b"".join([content for content in response.streaming_content]) + streamed_content = b"".join(list(response.streaming_content)) csv_file = StringIO(streamed_content.decode("utf8")) assert csv_file.read() == ( diff --git a/src/dashboard/src/components/filesystem_ajax/helpers.py b/src/dashboard/src/components/filesystem_ajax/helpers.py index e009ece42a..ff09a79a9d 100644 --- a/src/dashboard/src/components/filesystem_ajax/helpers.py +++ b/src/dashboard/src/components/filesystem_ajax/helpers.py @@ -29,7 +29,9 @@ def sorted_directory_list(path): return sorted(entries, key=helpers.keynat) -def directory_to_dict(path, directory={}, entry=False): +def directory_to_dict(path, directory=None, entry=False): + if directory is None: + directory = {} # if starting traversal, set entry to directory root if entry is False: entry = directory @@ -77,7 +79,7 @@ def check_filepath_exists(filepath): try: filepath.index("..") error = "Illegal path." - except: + except Exception: pass return error diff --git a/src/dashboard/src/components/helpers.py b/src/dashboard/src/components/helpers.py index 09b64af811..898fc014fa 100644 --- a/src/dashboard/src/components/helpers.py +++ b/src/dashboard/src/components/helpers.py @@ -178,7 +178,7 @@ def get_setting(setting, default=""): try: setting = models.DashboardSetting.objects.get(name=setting) return setting.value - except: + except Exception: return default @@ -193,7 +193,7 @@ def get_boolean_setting(setting, default=""): def set_setting(setting, value=""): try: setting_data = models.DashboardSetting.objects.get(name=setting) - except: + except Exception: setting_data = models.DashboardSetting.objects.create() setting_data.name = setting # ``DashboardSetting.value`` is a string-based field. The empty string is diff --git a/src/dashboard/src/components/ingest/pair_matcher.py b/src/dashboard/src/components/ingest/pair_matcher.py index 72f6cf0007..c6b25b8fc3 100644 --- a/src/dashboard/src/components/ingest/pair_matcher.py +++ b/src/dashboard/src/components/ingest/pair_matcher.py @@ -178,8 +178,10 @@ def match_dip_objects_to_resource_levels( parent_url, reset_url, uuid, - matches=[], + matches=None, ): + if matches is None: + matches = [] # load object relative paths object_path_json = json.JSONEncoder().encode( ingest_upload_atk_get_dip_object_paths(uuid) @@ -203,8 +205,10 @@ def match_dip_objects_to_resource_component_levels( parent_url, reset_url, uuid, - matches=[], + matches=None, ): + if matches is None: + matches = [] # load object relative paths object_path_json = json.JSONEncoder().encode( ingest_upload_atk_get_dip_object_paths(uuid) @@ -238,7 +242,9 @@ def remove_review_matches_prefixes(path): return remove_objects_prefix(remove_sip_dir_prefix(path)) -def review_matches(client, request, template, uuid, matches=[]): +def review_matches(client, request, template, uuid, matches=None): + if matches is None: + matches = [] object_paths = { file_.uuid: remove_review_matches_prefixes(file_.currentlocation.decode()) for file_ in models.File.objects.filter(sip=uuid) @@ -262,7 +268,7 @@ def ingest_upload_atk_get_dip_object_paths(uuid): dip_upload_dir = os.path.join(watch_dir, "uploadDIP") try: sip = models.SIP.objects.get(uuid=uuid) - except: + except Exception: raise Http404 directory = os.path.basename(os.path.dirname(sip.currentpath)) metsFilePath = os.path.join(dip_upload_dir, directory, "METS." + uuid + ".xml") diff --git a/src/dashboard/src/components/ingest/views.py b/src/dashboard/src/components/ingest/views.py index ffe2592dfe..40ca99f266 100644 --- a/src/dashboard/src/components/ingest/views.py +++ b/src/dashboard/src/components/ingest/views.py @@ -80,7 +80,7 @@ def _adjust_directories_draggability(nodes): def ingest_grid(request): try: storage_service.get_location(purpose="BL") - except: + except Exception: messages.warning( request, _( @@ -175,7 +175,7 @@ def ingest_metadata_edit(request, uuid, id=None): def ingest_metadata_add_files(request, sip_uuid): try: source_directories = storage_service.get_location(purpose="TS") - except: + except Exception: messages.warning( request, _( @@ -288,7 +288,7 @@ def ingest_metadata_delete(request, uuid, id): models.DublinCore.objects.get(pk=id).delete() messages.info(request, _("Deleted.")) return redirect("ingest:ingest_metadata_list", uuid) - except: + except Exception: raise Http404 @@ -322,7 +322,7 @@ def ingest_upload(request, uuid): if "target" in request.POST: try: access = models.Access.objects.get(sipuuid=uuid) - except: + except Exception: access = models.Access(sipuuid=uuid) access.target = pickle.dumps( {"target": request.POST["target"]}, protocol=0 @@ -334,7 +334,7 @@ def ingest_upload(request, uuid): try: access = models.Access.objects.get(sipuuid=uuid) data = pickle.loads(access.target.encode()) - except: + except Exception: raise Http404 return helpers.json_response(data) @@ -629,7 +629,7 @@ def transfer_backlog(request, ui): query = advanced_search.assemble_query( queries, ops, fields, types, filters=[backlog_filter] ) - except: + except Exception: logger.exception("Error accessing index.") return HttpResponse("Error accessing index.") @@ -638,7 +638,7 @@ def transfer_backlog(request, ui): results = elasticSearchFunctions.search_all_results( es_client, body=query, index="transferfiles" ) - except: + except Exception: logger.exception("Error accessing index.") return HttpResponse("Error accessing index.") @@ -693,7 +693,7 @@ def transfer_file_download(request, uuid): # get file basename try: file = models.File.objects.get(uuid=uuid) - except: + except Exception: raise Http404 shared_directory_path = django_settings.SHARED_DIRECTORY diff --git a/src/dashboard/src/components/rights/views.py b/src/dashboard/src/components/rights/views.py index c07261069b..6e28b0f9af 100644 --- a/src/dashboard/src/components/rights/views.py +++ b/src/dashboard/src/components/rights/views.py @@ -254,7 +254,7 @@ def rights_edit(request, uuid, id=None, section="ingest"): createdCopyright = models.RightsStatementCopyright.objects.get( rightsstatement=createdRights ) - except: + except Exception: createdCopyright = models.RightsStatementCopyright( rightsstatement=createdRights ) @@ -280,7 +280,7 @@ def rights_edit(request, uuid, id=None, section="ingest"): createdCopyright = models.RightsStatementCopyright.objects.get( rightsstatement=createdRights ) - except: + except Exception: createdCopyright = models.RightsStatementCopyright( rightsstatement=createdRights ) @@ -324,7 +324,7 @@ def rights_edit(request, uuid, id=None, section="ingest"): createdLicense = models.RightsStatementLicense.objects.get( rightsstatement=createdRights ) - except: + except Exception: createdLicense = models.RightsStatementLicense( rightsstatement=createdRights ) @@ -350,7 +350,7 @@ def rights_edit(request, uuid, id=None, section="ingest"): createdLicense = models.RightsStatementLicense.objects.get( rightsstatement=createdRights ) - except: + except Exception: createdLicense = models.RightsStatementLicense( rightsstatement=createdRights ) @@ -533,7 +533,7 @@ def rights_edit(request, uuid, id=None, section="ingest"): rightsstatement=createdRights ) ) - except: + except Exception: createdOther = models.RightsStatementOtherRightsInformation( rightsstatement=createdRights ) @@ -561,7 +561,7 @@ def rights_edit(request, uuid, id=None, section="ingest"): rightsstatement=createdRights ) ) - except: + except Exception: createdOther = models.RightsStatementOtherRightsInformation( rightsstatement=createdRights ) @@ -594,7 +594,7 @@ def rights_edit(request, uuid, id=None, section="ingest"): url = reverse("rights_%s:edit" % section, args=[uuid, createdRights.pk]) try: url = url + "?created=" + new_content_type_created - except: + except Exception: pass return redirect(url) else: @@ -751,7 +751,7 @@ def rights_holders_lookup(request, id): try: agent = models.RightsStatementLinkingAgentIdentifier.objects.get(pk=id) result = agent.linkingagentidentifiervalue + " [" + str(agent.id) + "]" - except: + except Exception: result = "" return HttpResponse(result) diff --git a/src/dashboard/src/components/transfer/views.py b/src/dashboard/src/components/transfer/views.py index d7dd2475ab..e0b15a8ddc 100644 --- a/src/dashboard/src/components/transfer/views.py +++ b/src/dashboard/src/components/transfer/views.py @@ -53,7 +53,7 @@ def grid(request): def transfer_source_locations(request): try: return helpers.json_response(storage_service.get_location(purpose="TS")) - except: + except Exception: message = _("Error retrieving source directories") logger.exception(message) response = {"message": message, "status": "Failure"} diff --git a/src/dashboard/src/fpr/migrations/0035_python3_compatibility.py b/src/dashboard/src/fpr/migrations/0035_python3_compatibility.py index eab9d87c7f..bcbbba14bb 100644 --- a/src/dashboard/src/fpr/migrations/0035_python3_compatibility.py +++ b/src/dashboard/src/fpr/migrations/0035_python3_compatibility.py @@ -535,7 +535,7 @@ def identify(file_): try: results = stdout.split('\n')[0].split(',') - except: + except Exception: raise FidoFailed(stdout, stderr, process.returncode) if process.returncode != 0 or results[-1] == '"fail"': diff --git a/src/dashboard/src/fpr/views.py b/src/dashboard/src/fpr/views.py index 9143a888e6..8c8050b474 100644 --- a/src/dashboard/src/fpr/views.py +++ b/src/dashboard/src/fpr/views.py @@ -730,7 +730,7 @@ def revision_list(request, entity_name, uuid): # restrict to models that are intended to have revisions try: - getattr(model, "replaces") + getattr(model, "replaces") # noqa: B009 # get specific revision's data and augment with detail URL revision = model.objects.get(uuid=uuid) diff --git a/src/dashboard/src/main/management/commands/rebuild_transfer_backlog.py b/src/dashboard/src/main/management/commands/rebuild_transfer_backlog.py index e2d6aa48d5..fc04f3797c 100644 --- a/src/dashboard/src/main/management/commands/rebuild_transfer_backlog.py +++ b/src/dashboard/src/main/management/commands/rebuild_transfer_backlog.py @@ -522,13 +522,11 @@ def _import_self_describing_transfer( """ transfer, created = Transfer.objects.get_or_create( uuid=transfer_uuid, - defaults=dict( - type="Standard", - diruuids=False, - currentlocation="%sharedPath%www/AIPsStore/transferBacklog/originals/" - + str(transfer_dir.name) - + "/", - ), + defaults={ + "type": "Standard", + "diruuids": False, + "currentlocation": f"%sharedPath%www/AIPsStore/transferBacklog/originals/{transfer_dir.name}/", + }, ) # The transfer did not exist, we need to populate everything else. diff --git a/src/dashboard/src/main/management/commands/resolve_pending_jobs.py b/src/dashboard/src/main/management/commands/resolve_pending_jobs.py index 3f7585a530..e3cafd0257 100644 --- a/src/dashboard/src/main/management/commands/resolve_pending_jobs.py +++ b/src/dashboard/src/main/management/commands/resolve_pending_jobs.py @@ -97,7 +97,7 @@ def loop(self, *args, **options): try: client.execute_unit(package_id, chain_id) break - except: + except Exception: self.error("There was a problem executing the selected choice") def admin_user(self): diff --git a/src/dashboard/src/main/models.py b/src/dashboard/src/main/models.py index 47b49abe0b..3a00882b87 100644 --- a/src/dashboard/src/main/models.py +++ b/src/dashboard/src/main/models.py @@ -131,12 +131,9 @@ def get_dict(self, scope): {u'foo': u'bar'} """ - return { - key: value - for (key, value) in self.get_queryset() - .filter(scope=scope) - .values_list("name", "value") - } + return dict( + self.get_queryset().filter(scope=scope).values_list("name", "value") + ) def set_dict(self, scope, items): """ diff --git a/src/dashboard/src/main/templatetags/breadcrumb.py b/src/dashboard/src/main/templatetags/breadcrumb.py index 51b68f3e7a..43213ac5bc 100644 --- a/src/dashboard/src/main/templatetags/breadcrumb.py +++ b/src/dashboard/src/main/templatetags/breadcrumb.py @@ -83,7 +83,7 @@ def render(self, context): try: val = self.vars[0] title = val.resolve(context) - except: + except Exception: title = "" else: @@ -115,7 +115,7 @@ def render(self, context): try: val = self.title title = val.resolve(context) - except: + except Exception: title = "" else: title = title.strip("'").strip('"') diff --git a/src/dashboard/tests/test_api.py b/src/dashboard/tests/test_api.py index 7650df79ff..c790feaf5a 100644 --- a/src/dashboard/tests/test_api.py +++ b/src/dashboard/tests/test_api.py @@ -517,10 +517,8 @@ def test_list_processing_configs(self): assert len(processing_configs) == 2 expected_names = sorted(["default", "automated"]) assert all( - [ - actual == expected - for actual, expected in zip(processing_configs, expected_names) - ] + actual == expected + for actual, expected in zip(processing_configs, expected_names) ) def test_get_existing_processing_config(self): diff --git a/src/dashboard/tests/test_filesystem_ajax.py b/src/dashboard/tests/test_filesystem_ajax.py index 6d87dc5a2a..a6eb3ec035 100644 --- a/src/dashboard/tests/test_filesystem_ajax.py +++ b/src/dashboard/tests/test_filesystem_ajax.py @@ -691,4 +691,4 @@ def test_copy_within_arrange(mocker, admin_client): # Verify SIPArrange instances were created as expected. assert models.SIPArrange.objects.count() == 2 - assert set(list(models.SIPArrange.objects.values_list(*attrs))) == expected + assert set(models.SIPArrange.objects.values_list(*attrs)) == expected diff --git a/src/dashboard/tests/test_transfer.py b/src/dashboard/tests/test_transfer.py index 310e7449fc..a7fce448b8 100644 --- a/src/dashboard/tests/test_transfer.py +++ b/src/dashboard/tests/test_transfer.py @@ -85,9 +85,7 @@ def test_component_post(admin_client): assert "Metadata saved." in response.content.decode() assert set( - list( - TransferMetadataFieldValue.objects.filter( - set=transfer_uuid, field__fieldname__in=["media_format", "media_number"] - ).values_list("fieldvalue") - ) + TransferMetadataFieldValue.objects.filter( + set=transfer_uuid, field__fieldname__in=["media_format", "media_number"] + ).values_list("fieldvalue") ) == {('3.5" floppy',), ("123",)} diff --git a/tox.ini b/tox.ini index e8facf4f3b..0c0187c133 100644 --- a/tox.ini +++ b/tox.ini @@ -84,6 +84,17 @@ commands = pre-commit run --all-files --show-diff-on-failure [flake8] exclude = .tox, .git, __pycache__, .cache, build, dist, *.pyc, *.egg-info, .eggs -application-import-names = flake8 -select = C, E, F, W, B, B950 -ignore = E203, E402, E501, E722, W503, W605 +# Error codes: +# - https://flake8.pycqa.org/en/latest/user/error-codes.html +# - https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes +# - https://github.com/PyCQA/flake8-bugbear#list-of-warnings +# +# E203: whitespace before ‘,’, ‘;’, or ‘:’ +# E402: module level import not at top of file +# E501: line too long +# W503: line break before binary operator +ignore = + E203, + E402, + E501, + W503