diff --git a/Collector/Collector.py b/Collector/Collector.py index 0cce2670..62b0b4d1 100755 --- a/Collector/Collector.py +++ b/Collector/Collector.py @@ -83,7 +83,7 @@ def refreshFromZip(self, zipFileName: str) -> None: # Now clean the signature directory, only deleting signatures and metadata for sigFile in os.listdir(self.sigCacheDir): - if sigFile.endswith(".signature") or sigFile.endswith(".metadata"): + if sigFile.endswith((".signature", ".metadata")): os.remove(os.path.join(self.sigCacheDir, sigFile)) else: print( @@ -210,16 +210,15 @@ def search(self, crashInfo: CrashInfo) -> tuple[str | None, dict[str, Any] | Non sigFile = os.path.join(self.sigCacheDir, sigFile) if not os.path.isdir(sigFile): with open(sigFile) as f: - sigData = f.read() - crashSig = CrashSignature(sigData) - if crashSig.matches(crashInfo): - metadataFile = sigFile.replace(".signature", ".metadata") - metadata: dict[str, Any] | None = None - if os.path.exists(metadataFile): - with open(metadataFile) as m: - metadata = json.loads(m.read()) + crashSig = CrashSignature(f.read()) + if crashSig.matches(crashInfo): + metadataFile = sigFile.replace(".signature", ".metadata") + metadata: dict[str, Any] | None = None + if os.path.exists(metadataFile): + with open(metadataFile) as m: + metadata = json.load(m) - return (sigFile, metadata) + return (sigFile, metadata) return (None, None) @@ -772,7 +771,7 @@ def main(args: list[str] | None = None) -> int: "Command line arguments:", " ".join(args), ) - print("") + print() if retJSON.get("env"): env = json.loads(retJSON["env"]) @@ -780,14 +779,14 @@ def main(args: list[str] | None = None) -> int: "Environment variables:", " ".join(f"{k} = {v}" for (k, v) in env.items()), ) - print("") + print() if retJSON.get("metadata"): metadata = json.loads(retJSON["metadata"]) print("== Metadata ==") for k, v in metadata.items(): print(f"{k} = {v}") - print("") + print() print(retFile) return 0 diff --git a/FTB/AssertionHelper.py b/FTB/AssertionHelper.py index df5e260e..a3ae9387 100644 --- a/FTB/AssertionHelper.py +++ b/FTB/AssertionHelper.py @@ -282,7 +282,11 @@ def getSanitizedAssertionPattern(msgs: str | list[str]) -> str | list[str]: matchPattern if replacementText is None else replacementText ) - def _handleMatch(match: re.Match[str]) -> str: + def _handleMatch( + match: re.Match[str], + replacementPattern: str = replacementPattern, + bsPositions: list[int] = bsPositions, + ) -> str: start = match.start(0) end = match.end(0) lengthDiff = len(replacementPattern) - len(match.group(0)) diff --git a/FTB/CoverageHelper.py b/FTB/CoverageHelper.py index 435b3547..8c07ee59 100644 --- a/FTB/CoverageHelper.py +++ b/FTB/CoverageHelper.py @@ -74,7 +74,7 @@ def merge_recursive(r: dict[str, Any], s: dict[str, Any]) -> None: minlen = min(len(rc), len(sc)) - for idx in range(0, minlen): + for idx in range(minlen): # There are multiple situations where coverage reports might disagree # about which lines are coverable and which are not. Sometimes, GCOV # reports this wrong in headers, but it can also happen when mixing diff --git a/FTB/Running/GDB.py b/FTB/Running/GDB.py index 814d1e94..012729c3 100644 --- a/FTB/Running/GDB.py +++ b/FTB/Running/GDB.py @@ -87,7 +87,7 @@ def printImportantRegisters() -> None: ] elif isARM64(): # ARM64 has GPRs from x0 to x30 - regs = ["x" + str(x) for x in range(0, 31)] + regs = ["x" + str(x) for x in range(31)] regs.extend(["sp", "pc", "cpsr", "fpcsr", "fpcr"]) else: regs = ["eax", "ebx", "ecx", "edx", "esi", "edi", "ebp", "esp", "eip"] diff --git a/FTB/Running/PersistentApplication.py b/FTB/Running/PersistentApplication.py index 430c507a..5331414f 100644 --- a/FTB/Running/PersistentApplication.py +++ b/FTB/Running/PersistentApplication.py @@ -23,7 +23,7 @@ import signal import subprocess import time -from abc import ABCMeta +from abc import ABCMeta, abstractmethod from enum import Enum, IntEnum, auto from FTB.Running.StreamCollector import StreamCollector @@ -119,16 +119,19 @@ def __init__( self.spfpPrefix = "" self.spfpSuffix = "" # To support + @abstractmethod def start(self, test: str | None = None) -> int | None: pass + @abstractmethod def stop(self) -> None: pass + @abstractmethod def runTest(self, test: str) -> int | None: pass - def status(self) -> int | None: + def status(self) -> int | None: # noqa: B027 pass def _crashed(self) -> bool: @@ -278,17 +281,17 @@ def start(self, test: str | None = None) -> int | None: f"{self.spfpPrefix}spfp-selftest{self.spfpSuffix}", file=self.process.stdin, ) - except OSError: + except OSError as exc: raise RuntimeError( "SPFP Error: Selftest failed, application did not start properly." - ) + ) from exc try: response = self.responseQueue.get( block=True, timeout=self.processingTimeout ) - except queue.Empty: - raise RuntimeError("SPFP Error: Selftest failed, no response.") + except queue.Empty as exc: + raise RuntimeError("SPFP Error: Selftest failed, no response.") from exc if response != "PASSED": raise RuntimeError( @@ -362,7 +365,7 @@ def runTest(self, test: str) -> int | None: response = self.responseQueue.get( block=True, timeout=self.processingTimeout ) - except queue.Empty: + except queue.Empty as exc: if self.process.poll() is None: # The process is still running, force it to stop and return timeout # code @@ -384,14 +387,14 @@ def runTest(self, test: str) -> int | None: raise RuntimeError( "SPFP Error: Application terminated with signal: " f"{self.process.returncode}" - ) + ) from exc # The application exited, but didn't send us any message before # doing so. We consider this a protocol violation and raise an # exception. raise RuntimeError( "SPFP Error: Application exited without message. " f"Exitcode: {self.process.returncode}" - ) + ) from exc # Update stdout/err available for the last run assert self.outCollector is not None diff --git a/FTB/Running/tests/test_persistent.py b/FTB/Running/tests/test_persistent.py index a08ba1c1..781ee9c7 100644 --- a/FTB/Running/tests/test_persistent.py +++ b/FTB/Running/tests/test_persistent.py @@ -150,7 +150,7 @@ def _check(spa): oldPid = spa.process.pid startTime = time.time() - for i in range(1, 10000): + for _ in range(1, 10000): spa.runTest("aaa\naaaa") stopTime = time.time() diff --git a/FTB/Signatures/CrashInfo.py b/FTB/Signatures/CrashInfo.py index 22c98bdb..2ea5d6a7 100644 --- a/FTB/Signatures/CrashInfo.py +++ b/FTB/Signatures/CrashInfo.py @@ -155,7 +155,7 @@ def __init__(self, *args: Any, **kwds: Any) -> None: super().__init__(*args, **kwds) -class CrashInfo(metaclass=ABCMeta): +class CrashInfo(metaclass=ABCMeta): # noqa: B024 """ Abstract base class that provides a method to instantiate the right sub class. It also supports generating a CrashSignature based on the stored information. @@ -525,7 +525,7 @@ def createCrashSignature( # StackFramesSymptom is only supported in 1.2 and higher, # for everything else, use multiple stackFrame symptoms if minimumSupportedVersion < 12: - for idx in range(0, numFrames): + for idx in range(numFrames): functionName = self.backtrace[idx] if functionName != "??": symptomArr.append( @@ -541,7 +541,7 @@ def createCrashSignature( else: framesArray: list[str] = [] - for idx in range(0, numFrames): + for idx in range(numFrames): functionName = self.backtrace[idx] if functionName != "??": framesArray.append(functionName) @@ -552,7 +552,7 @@ def createCrashSignature( topStackMissCount += 1 lastSymbolizedFrame = None - for frameIdx in range(0, len(framesArray)): + for frameIdx in range(len(framesArray)): if framesArray[frameIdx] != "?": lastSymbolizedFrame = frameIdx @@ -712,7 +712,7 @@ def __init__( self.configuration = configuration # If crashData is given, use that to find the ASan trace, otherwise use stderr - asanOutput = crashData if crashData else stderr + asanOutput = crashData or stderr assert asanOutput is not None asanCrashAddressPattern = r"""(?x) @@ -951,7 +951,7 @@ def __init__( self.configuration = configuration # If crashData is given, use that to find the LSan trace, otherwise use stderr - lsanOutput = crashData if crashData else stderr + lsanOutput = crashData or stderr assert lsanOutput is not None lsanErrorPattern = "ERROR: LeakSanitizer:" lsanPatternSeen = False @@ -1048,7 +1048,7 @@ def __init__( self.configuration = configuration # If crashData is given, use that to find the UBSan trace, otherwise use stderr - ubsanOutput = crashData if crashData else stderr + ubsanOutput = crashData or stderr assert ubsanOutput is not None ubsanErrorPattern = r":\d+:\d+:\s+runtime\s+error:\s+" ubsanPatternSeen = False @@ -1538,7 +1538,7 @@ def calculateARMDerefOpAddress( # ARM assembly has nested comma-separated operands, so we need to merge # those inside brackets back together before proceeding. - for i in range(0, len(parts)): + for i in range(len(parts)): if i >= len(parts): break if ( @@ -1559,9 +1559,7 @@ def calculateARMDerefOpAddress( if instruction == "brk": # This is an explicit breakpoint / trap return RegisterHelper.getInstructionPointer(registerMap) - elif len(parts) == 2 and ( - instruction.startswith("ldr") or instruction.startswith("str") - ): + elif len(parts) == 2 and (instruction.startswith(("ldr", "str"))): # Load/Store instruction match = re.match("^\\s*\\[(.*)\\]$", parts[1]) if match is not None: @@ -1991,7 +1989,7 @@ def __init__( self.configuration = configuration # If crashData is given, use that to find the ASan trace, otherwise use stderr - tsanOutput = crashData if crashData else stderr + tsanOutput = crashData or stderr assert tsanOutput is not None tsanWarningPattern = r"""WARNING: ThreadSanitizer:.*\s.+?\s+\(pid=\d+\)""" @@ -2160,7 +2158,7 @@ def __init__( # If crashData is given, use that to find the Valgrind trace, otherwise use # stderr - vgdOutput = crashData if crashData else stderr + vgdOutput = crashData or stderr assert vgdOutput is not None stackPattern = re.compile( r""" @@ -2211,7 +2209,7 @@ def createShortSignature(self) -> str: @return: A string representing this crash (short signature) """ - logData = self.rawCrashData if self.rawCrashData else self.rawStderr + logData = self.rawCrashData or self.rawStderr for line in logData: m = re.match(ValgrindCrashInfo.MSG_REGEX, line) if m and m.group("msg"): diff --git a/FTB/Signatures/CrashSignature.py b/FTB/Signatures/CrashSignature.py index 5afe759a..69eb4788 100644 --- a/FTB/Signatures/CrashSignature.py +++ b/FTB/Signatures/CrashSignature.py @@ -53,7 +53,7 @@ def __init__(self, rawSignature: str) -> None: try: obj = json.loads(rawSignature) except ValueError as e: - raise RuntimeError(f"Invalid JSON: {e}") + raise RuntimeError(f"Invalid JSON: {e}") from e # Get the symptoms objects (mandatory) if "symptoms" in obj: @@ -254,12 +254,7 @@ def getSignatureUnifiedDiffTuples( signatureDiff = difflib.unified_diff(oldLines, newLines, n=context) for diffLine in signatureDiff: - if ( - diffLine.startswith("+++") - or diffLine.startswith("---") - or diffLine.startswith("@@") - or not diffLine.strip() - ): + if diffLine.startswith(("+++", "---", "@@")) or not diffLine.strip(): continue diffTuples.append((diffLine[0], diffLine[1:])) diff --git a/FTB/Signatures/Matchers.py b/FTB/Signatures/Matchers.py index ba15dd5a..6bb0c731 100644 --- a/FTB/Signatures/Matchers.py +++ b/FTB/Signatures/Matchers.py @@ -48,7 +48,7 @@ def __init__(self, obj: str | bytes | dict[str, Any]) -> None: try: self.compiledValue = re.compile(self.value) except re.error as e: - raise RuntimeError(f"Error in regular expression: {e}") + raise RuntimeError(f"Error in regular expression: {e}") from e else: value = JSONHelper.getStringChecked(obj, "value", True) assert value is not None @@ -63,7 +63,7 @@ def __init__(self, obj: str | bytes | dict[str, Any]) -> None: try: self.compiledValue = re.compile(self.value) except re.error as e: - raise RuntimeError(f"Error in regular expression: {e}") + raise RuntimeError(f"Error in regular expression: {e}") from e else: raise RuntimeError(f"Unknown match operator specified: {matchType}") @@ -122,19 +122,19 @@ def __init__(self, obj: str | bytes | int) -> None: matchType = numberMatchComponents[0] try: self.matchType = NumberMatchType(matchType) - except ValueError: + except ValueError as exc: raise RuntimeError( f"Unknown match operator specified: {matchType}" - ) + ) from exc try: value = numberMatchComponents[numIdx] base = 16 if value.startswith("0x") else 10 self.value = int(numberMatchComponents[numIdx], base) - except ValueError: + except ValueError as exc: raise RuntimeError( f"Invalid number specified: {numberMatchComponents[numIdx]}" - ) + ) from exc else: # We're trying to match the fact that we cannot calculate a crash # address diff --git a/FTB/Signatures/RegisterHelper.py b/FTB/Signatures/RegisterHelper.py index f9f895c2..2989bc44 100644 --- a/FTB/Signatures/RegisterHelper.py +++ b/FTB/Signatures/RegisterHelper.py @@ -56,7 +56,7 @@ "cpsr", ] -arm64Registers = ["x" + str(x) for x in range(0, 31)] + [ +arm64Registers = ["x" + str(x) for x in range(31)] + [ "sp", "pc", "cpsr", diff --git a/Reporter/Reporter.py b/Reporter/Reporter.py index f861a766..01860bfc 100644 --- a/Reporter/Reporter.py +++ b/Reporter/Reporter.py @@ -153,7 +153,7 @@ def sentry_init() -> None: sentry_fuzzing_config.init() -class Reporter(ABC): +class Reporter(ABC): # noqa: B024 def __init__( self, sigCacheDir: str | None = None, diff --git a/TaskStatusReporter/TaskStatusReporter.py b/TaskStatusReporter/TaskStatusReporter.py index f2057650..2cf919d2 100755 --- a/TaskStatusReporter/TaskStatusReporter.py +++ b/TaskStatusReporter/TaskStatusReporter.py @@ -39,9 +39,8 @@ class TaskStatusReporter(Reporter): @functools.wraps(Reporter.__init__) def __init__(self, *args, **kwargs): - kwargs.setdefault( - "tool", "N/A" - ) # tool is required by remote_checks, but unused by TaskStatusReporter + # 'tool' is required by remote_checks, but unused by TaskStatusReporter + kwargs.setdefault("tool", "N/A") super().__init__(*args, **kwargs) @staticmethod diff --git a/misc/afl-libfuzzer/afl-libfuzzer-daemon.py b/misc/afl-libfuzzer/afl-libfuzzer-daemon.py index da6a4dfe..dcae2027 100755 --- a/misc/afl-libfuzzer/afl-libfuzzer-daemon.py +++ b/misc/afl-libfuzzer/afl-libfuzzer-daemon.py @@ -240,8 +240,7 @@ def write_stats_file(outfile, fields, stats, warnings): f.write(f"{field}{' ' * (max_keylen + 1 - len(field))}: {val}\n") - for warning in warnings: - f.write(warning) + f.writelines(warnings) return @@ -531,7 +530,7 @@ def scan_crashes( crash_file = os.path.join(crash_dir, crash_file) # Ignore our own status files - if crash_file.endswith(".submitted") or crash_file.endswith(".failed"): + if crash_file.endswith((".submitted", ".failed")): continue # Ignore files we already processed @@ -682,8 +681,10 @@ def apply_transform(script_path, testcase_path): with tempfile.TemporaryDirectory() as output_path: try: subprocess.check_call([script_path, testcase_path, output_path]) - except subprocess.CalledProcessError: - raise Exception("Failed to apply post crash transformation. Aborting...") + except subprocess.CalledProcessError as exc: + raise Exception( + "Failed to apply post crash transformation. Aborting..." + ) from exc if len(os.listdir(output_path)) == 0: raise Exception( @@ -1450,7 +1451,7 @@ def warn_local(): return 2 for arg in cmdline: - if arg.startswith("-jobs=") or arg.startswith("-workers="): + if arg.startswith(("-jobs=", "-workers=")): print( "Error: Using -jobs and -workers is incompatible with this " "wrapper.", @@ -1789,9 +1790,7 @@ def warn_local(): testcase_name = os.path.basename(testcase) if not monitor.inited: - if testcase_name.startswith("oom-") or testcase_name.startswith( - "timeout-" - ): + if testcase_name.startswith(("oom-", "timeout-")): hashname = testcase_name.split("-")[1] potential_corpus_file = os.path.join(corpus_dir, hashname) if os.path.exists(potential_corpus_file): diff --git a/misc/ec2prices/simulator.py b/misc/ec2prices/simulator.py index 744ad137..8a9bacb5 100644 --- a/misc/ec2prices/simulator.py +++ b/misc/ec2prices/simulator.py @@ -64,7 +64,6 @@ def get_spot_price_per_region( break except Exception: print("Caught exception, retrying") - pass return r @@ -272,7 +271,7 @@ def main(): total_price = sim_module.run(priceData, simulation, configFile.main) results[simulation_name] = total_price - print("") + print() col_len = None @@ -284,7 +283,7 @@ def main(): col_len += 1 - print("") + print() sys.stdout.write(" " * col_len) for simulation in results: diff --git a/pyproject.toml b/pyproject.toml index 718f6c49..136cdb4b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,6 +50,8 @@ target-version = "py310" [tool.ruff.lint] select = [ + # flake8-bugbear + "B", # flake8-comprehensions "C4", # pycodestyle @@ -60,10 +62,20 @@ select = [ "FA", # Flynt "FLY", + # refurb + "FURB", + # flake8-logging-format + "G", # isort "I", + # flake8-logging + "LOG", # Perflint "PERF", + # flake8-pie + "PIE", + # flake8-quotes + "Q", # flake8-return "RET", # Ruff-specific rules diff --git a/server/covmanager/SourceCodeProvider/GITSourceCodeProvider.py b/server/covmanager/SourceCodeProvider/GITSourceCodeProvider.py index 90acd73a..fb96567a 100644 --- a/server/covmanager/SourceCodeProvider/GITSourceCodeProvider.py +++ b/server/covmanager/SourceCodeProvider/GITSourceCodeProvider.py @@ -33,10 +33,10 @@ def getSource(self, filename, revision): except subprocess.CalledProcessError: # Check if the revision exists to determine which exception to raise if not self.testRevision(revision): - raise UnknownRevisionException + raise UnknownRevisionException from None # Otherwise assume the file doesn't exist - raise UnknownFilenameException + raise UnknownFilenameException from None def testRevision(self, revision): try: @@ -57,7 +57,7 @@ def getParents(self, revision): ["git", "log", revision, "--format=%P"], cwd=self.location ) except subprocess.CalledProcessError: - raise UnknownRevisionException + raise UnknownRevisionException from None output = output.decode("utf-8").splitlines() diff --git a/server/covmanager/SourceCodeProvider/HGSourceCodeProvider.py b/server/covmanager/SourceCodeProvider/HGSourceCodeProvider.py index f2dc5c99..96515741 100644 --- a/server/covmanager/SourceCodeProvider/HGSourceCodeProvider.py +++ b/server/covmanager/SourceCodeProvider/HGSourceCodeProvider.py @@ -30,8 +30,7 @@ def getSource(self, filename, revision): revision = revision.replace("+", "") # Avoid passing in absolute filenames to HG - if filename.startswith("/"): - filename = filename[1:] + filename = filename.removeprefix("/") try: return subprocess.check_output( @@ -40,10 +39,10 @@ def getSource(self, filename, revision): except subprocess.CalledProcessError: # Check if the revision exists to determine which exception to raise if not self.testRevision(revision): - raise UnknownRevisionException + raise UnknownRevisionException from None # Otherwise assume the file doesn't exist - raise UnknownFilenameException + raise UnknownFilenameException from None def testRevision(self, revision): revision = revision.replace("+", "") @@ -71,7 +70,7 @@ def getParents(self, revision): cwd=self.location, ).decode("utf-8") except subprocess.CalledProcessError: - raise UnknownRevisionException + raise UnknownRevisionException from None output = output.splitlines() @@ -92,7 +91,7 @@ def getUnifiedDiff(self, revision): ["hg", "diff", "--git", "-U0", "-c", revision], cwd=self.location ) except subprocess.CalledProcessError: - raise UnknownRevisionException + raise UnknownRevisionException from None return output.decode("utf-8") diff --git a/server/covmanager/management/commands/setup_repository.py b/server/covmanager/management/commands/setup_repository.py index 58209be3..4d94eba5 100644 --- a/server/covmanager/management/commands/setup_repository.py +++ b/server/covmanager/management/commands/setup_repository.py @@ -35,7 +35,7 @@ def handle(self, name, provider, location, **opts): except ImportError: raise CommandError( f"Error: '{provider}' is not a valid source code provider!" - ) + ) from None if not location: raise CommandError("Error: invalid location") diff --git a/server/covmanager/views.py b/server/covmanager/views.py index 783c8d31..b3eb183e 100644 --- a/server/covmanager/views.py +++ b/server/covmanager/views.py @@ -385,7 +385,7 @@ def collections_patch_api(request, collectionid, patch_revision): missed_locations = [] not_coverable = [] - for idx in range(0, len(locations)): + for idx in range(len(locations)): location = locations[idx] if location > 0 and location < len(coverage): if coverage[location] == 0: diff --git a/server/crashmanager/management/commands/add_tool_to_user.py b/server/crashmanager/management/commands/add_tool_to_user.py index 6e2f62e8..fd6d238b 100644 --- a/server/crashmanager/management/commands/add_tool_to_user.py +++ b/server/crashmanager/management/commands/add_tool_to_user.py @@ -23,7 +23,7 @@ def handle(self, *args, **options): try: django_user = DjangoUser.objects.get(username=username) except DjangoUser.DoesNotExist: - raise CommandError(f"No user found with username '{username}'") + raise CommandError(f"No user found with username '{username}'") from None crash_manager_user = CrashManagerUser.get_or_create_restricted(django_user)[0] diff --git a/server/crashmanager/management/commands/rm_tool_from_user.py b/server/crashmanager/management/commands/rm_tool_from_user.py index 1025dc27..cb0f6e07 100644 --- a/server/crashmanager/management/commands/rm_tool_from_user.py +++ b/server/crashmanager/management/commands/rm_tool_from_user.py @@ -23,7 +23,7 @@ def handle(self, *args, **options): try: django_user = DjangoUser.objects.get(username=username) except DjangoUser.DoesNotExist: - raise CommandError(f"No user found with username '{username}'") + raise CommandError(f"No user found with username '{username}'") from None crash_manager_user = CrashManagerUser.get_or_create_restricted(django_user)[0] @@ -32,7 +32,7 @@ def handle(self, *args, **options): except Tool.DoesNotExist: raise CommandError( f"Error: Tool '{tool_name}' is not present in the database" - ) + ) from None crash_manager_user.defaultToolsFilter.remove(tool) diff --git a/server/crashmanager/tests/test_bugproviders_rest.py b/server/crashmanager/tests/test_bugproviders_rest.py index caf2d776..e12f53b1 100644 --- a/server/crashmanager/tests/test_bugproviders_rest.py +++ b/server/crashmanager/tests/test_bugproviders_rest.py @@ -97,5 +97,5 @@ def test_rest_bugproviders_list(api_client, user, cm): assert resp["next"] is None assert resp["previous"] is None assert len(resp["results"]) == expected - for result, provider in zip(resp["results"], providers[:expected]): + for result, provider in zip(resp["results"], providers[:expected], strict=True): _compare_rest_result_to_bugprovider(result, provider) diff --git a/server/crashmanager/tests/test_crashes_rest.py b/server/crashmanager/tests/test_crashes_rest.py index 27e94389..202049e7 100644 --- a/server/crashmanager/tests/test_crashes_rest.py +++ b/server/crashmanager/tests/test_crashes_rest.py @@ -253,7 +253,7 @@ def test_rest_crashes_list(api_client, user, cm, ignore_toolfilter, include_raw) assert resp["next"] is None assert resp["previous"] is None assert len(resp["results"]) == expected - for result, crash in zip(resp["results"], crashes[:expected]): + for result, crash in zip(resp["results"], crashes[:expected], strict=True): _compare_rest_result_to_crash(result, crash, raw=include_raw) diff --git a/server/crashmanager/tests/test_signatures_rest.py b/server/crashmanager/tests/test_signatures_rest.py index 6ca13c6b..d7aa6ebd 100644 --- a/server/crashmanager/tests/test_signatures_rest.py +++ b/server/crashmanager/tests/test_signatures_rest.py @@ -78,8 +78,10 @@ def _compare_rest_result_to_bucket( - result, bucket, size, quality, best_entry=None, hist=[], vue=False + result, bucket, size, quality, best_entry=None, hist=None, vue=False ): + if hist is None: + hist = [] attributes = { "best_entry", "best_quality", @@ -532,7 +534,7 @@ def test_new_signature_preview(api_client, cm, user, many): # pylint: disable=i assert len(in_list) == 100 assert data["inListCount"] == 201 - for shown, crash in zip(reversed(data["inList"]), crashes[-100:]): + for shown, crash in zip(reversed(data["inList"]), crashes[-100:], strict=True): assert shown["id"] == crash.pk else: crash = CrashEntry.objects.get(pk=crash.pk) # re-read @@ -778,9 +780,9 @@ def test_edit_signature_edit_preview(api_client, cm, user, many): # pylint: dis for crash in crashes2: crash = CrashEntry.objects.get(pk=crash.pk) # re-read assert crash.bucket is None - for shown, crash in zip(reversed(in_list), crashes2[-100:]): + for shown, crash in zip(reversed(in_list), crashes2[-100:], strict=True): assert shown["id"] == crash.pk - for shown, crash in zip(reversed(out_list), crashes1[-100:]): + for shown, crash in zip(reversed(out_list), crashes1[-100:], strict=True): assert shown["id"] == crash.pk else: assert len(in_list) == 1 diff --git a/server/crashmanager/tests/test_templates_rest.py b/server/crashmanager/tests/test_templates_rest.py index ea5b6562..71fa2a99 100644 --- a/server/crashmanager/tests/test_templates_rest.py +++ b/server/crashmanager/tests/test_templates_rest.py @@ -128,7 +128,7 @@ def test_rest_templates_list(api_client, user, cm): assert resp["next"] is None assert resp["previous"] is None assert len(resp["results"]) == expected - for result, template in zip(resp["results"], templates[:expected]): + for result, template in zip(resp["results"], templates[:expected], strict=True): _compare_rest_result_to_template(result, template) diff --git a/server/crashmanager/views.py b/server/crashmanager/views.py index 4f29736c..892d6896 100644 --- a/server/crashmanager/views.py +++ b/server/crashmanager/views.py @@ -406,7 +406,7 @@ def newSignature(request): if "stackframes" in request.GET: maxStackFrames = int(request.GET["stackframes"]) elif any( - entry.startswith("std::panicking") or entry.startswith("alloc::alloc") + entry.startswith(("std::panicking", "alloc::alloc")) for entry in crashInfo.backtrace ): # rust panic adds 5-6 frames of noise at the top of the stack @@ -923,11 +923,11 @@ def filter_queryset(self, request, queryset, view): try: _, queryobj = json_to_query(querystr) except (RuntimeError, TypeError) as e: - raise InvalidArgumentException(f"error in query: {e}") + raise InvalidArgumentException(f"error in query: {e}") from e try: queryset = queryset.filter(queryobj) except FieldError as exc: - raise InvalidArgumentException(f"error in query: {exc}") + raise InvalidArgumentException(f"error in query: {exc}") from exc return queryset @@ -944,7 +944,9 @@ def filter_queryset(self, request, queryset, view): ignore_toolfilter = int(ignore_toolfilter) assert ignore_toolfilter in {0, 1} except (AssertionError, ValueError): - raise InvalidArgumentException({"ignore_toolfilter": ["Expecting 0 or 1."]}) + raise InvalidArgumentException( + {"ignore_toolfilter": ["Expecting 0 or 1."]} + ) from None view.ignore_toolfilter = bool(ignore_toolfilter) return filter_crash_entries_by_toolfilter( request, @@ -977,7 +979,9 @@ def filter_queryset(self, request, queryset, view): ignore_toolfilter = int(ignore_toolfilter) assert ignore_toolfilter in {0, 1} except (AssertionError, ValueError): - raise InvalidArgumentException({"ignore_toolfilter": ["Expecting 0 or 1."]}) + raise InvalidArgumentException( + {"ignore_toolfilter": ["Expecting 0 or 1."]} + ) from None view.ignore_toolfilter = bool(ignore_toolfilter) return filter_signatures_by_toolfilter( request, @@ -1027,7 +1031,9 @@ def filter_queryset(self, request, queryset, view): include_raw = int(include_raw) assert include_raw in {0, 1} except (AssertionError, ValueError): - raise InvalidArgumentException({"include_raw": ["Expecting 0 or 1."]}) + raise InvalidArgumentException( + {"include_raw": ["Expecting 0 or 1."]} + ) from None if not include_raw: queryset = queryset.defer("rawStdout", "rawStderr", "rawCrashData") @@ -1135,7 +1141,7 @@ def partial_update(self, request, pk=None): try: obj = CrashEntry.objects.get(pk=pk) except CrashEntry.DoesNotExist: - raise Http404 + raise Http404 from None given_fields = set(request.data.keys()) disallowed_fields = given_fields - allowed_fields if disallowed_fields: @@ -1149,8 +1155,8 @@ def partial_update(self, request, pk=None): raise InvalidArgumentException("crash has no testcase") try: testcase_quality = int(request.data["testcase_quality"]) - except ValueError: - raise InvalidArgumentException("invalid testcase_quality") + except ValueError as exc: + raise InvalidArgumentException("invalid testcase_quality") from exc # NB: if other fields are added, all validation should occur before any DB # writes. obj.testcase.quality = testcase_quality @@ -1309,7 +1315,7 @@ def _validate(bucket, submit_save, reassign, limit, offset, created): try: bucket.getSignature() except RuntimeError as e: - raise ValidationError(f"Signature is not valid: {e}") + raise ValidationError(f"Signature is not valid: {e}") from e # Only save if we hit "save" (not e.g. "preview") # If offset is set, don't do it again (already done on first iteration) @@ -1537,7 +1543,7 @@ def json_to_query(json_str): try: obj = json.loads(json_str, object_pairs_hook=OrderedDict) except ValueError as e: - raise RuntimeError(f"Invalid JSON: {e}") + raise RuntimeError(f"Invalid JSON: {e}") from e def get_query_obj(obj, key=None): if obj is None or isinstance(obj, (str, list, int)): diff --git a/server/ec2spotmanager/CloudProvider/CloudProvider.py b/server/ec2spotmanager/CloudProvider/CloudProvider.py index d410bada..2b0ed8d9 100644 --- a/server/ec2spotmanager/CloudProvider/CloudProvider.py +++ b/server/ec2spotmanager/CloudProvider/CloudProvider.py @@ -59,15 +59,15 @@ def wrapper(*args, **kwds): return wrapped(*args, **kwds) except (ssl.SSLError, OSError) as exc: logging.getLogger("ec2spotmanager").exception("") - raise CloudProviderTemporaryFailure(f"{wrapped.__name__}: {exc}") + raise CloudProviderTemporaryFailure(f"{wrapped.__name__}: {exc}") from exc except CloudProviderError: logging.getLogger("ec2spotmanager").exception("") raise - except Exception: + except Exception as exc: logging.getLogger("ec2spotmanager").exception("") raise CloudProviderError( f"{wrapped.__name__}: unhandled error: {traceback.format_exc()}" - ) + ) from exc return wrapper diff --git a/server/ec2spotmanager/CloudProvider/EC2SpotCloudProvider.py b/server/ec2spotmanager/CloudProvider/EC2SpotCloudProvider.py index e27b87e4..9ce871c0 100644 --- a/server/ec2spotmanager/CloudProvider/EC2SpotCloudProvider.py +++ b/server/ec2spotmanager/CloudProvider/EC2SpotCloudProvider.py @@ -140,7 +140,7 @@ def start_instances( ) raise CloudProviderInstanceCountError( "Auto-selected region exceeded its maximum spot instance count." - ) + ) from msg if code == "RequestLimitExceeded": self.logger.warning( "Request limit exceeded for region %s, trying again later.", @@ -148,11 +148,11 @@ def start_instances( ) raise CloudProviderTemporaryFailure( f"Request limit exceeded for region {region}" - ) + ) from msg if code in {"InternalError", "Unavailable"}: raise CloudProviderTemporaryFailure( f"start_instances in region {region}: {msg}" - ) + ) from msg raise @wrap_provider_errors @@ -170,10 +170,10 @@ def check_instances_requests(self, region, instances, tags): ) raise CloudProviderTemporaryFailure( f"Request limit exceeded for region {region}" - ) + ) from msg raise - for req_id, result in zip(instances, results): + for req_id, result in zip(instances, results, strict=True): if isinstance(result, boto.ec2.instance.Instance): # state_code is a 16-bit value where the high byte is # an opaque internal value and should be ignored. @@ -232,7 +232,10 @@ def check_instances_requests(self, region, instances, tags): ) else: # state=failed self.logger.error( - f"Request {req_id} is {result.status.code} and {result.state}." + "Request %s is %s and %s.", + req_id, + result.status.code, + result.state, ) failed_requests[req_id] = {} failed_requests[req_id]["action"] = "disable_pool" @@ -267,7 +270,7 @@ def check_instances_state(self, pool_id, region): if code == "Unavailable": raise CloudProviderTemporaryFailure( f"getting instances in region {region}: {msg}" - ) + ) from msg raise for instance in boto_instances: @@ -357,7 +360,7 @@ def get_prices_per_region(self, region_name, instance_types=None): .append(float(price["SpotPrice"])) ) except botocore.exceptions.EndpointConnectionError as exc: - raise RuntimeError(f"Boto connection error: {exc}") + raise RuntimeError(f"Boto connection error: {exc}") from exc return prices diff --git a/server/ec2spotmanager/CloudProvider/GCECloudProvider.py b/server/ec2spotmanager/CloudProvider/GCECloudProvider.py index 18396b41..8ffaef0d 100644 --- a/server/ec2spotmanager/CloudProvider/GCECloudProvider.py +++ b/server/ec2spotmanager/CloudProvider/GCECloudProvider.py @@ -276,7 +276,7 @@ def check_instances_state(self, pool_id, region): ) except Exception as exc: if "Please try again" in str(exc): - raise CloudProviderTemporaryFailure(str(exc)) + raise CloudProviderTemporaryFailure(str(exc)) from exc raise for node in libcloud_nodes: @@ -423,7 +423,7 @@ def _get_skus_paginated(): if what is None: continue - if what.endswith("-cpu") or what.endswith("-inst"): + if what.endswith(("-cpu", "-inst")): assert unit == "h" else: assert what.endswith("-ram") @@ -443,9 +443,7 @@ def _get_skus_paginated(): mem = RAM_PER_INSTANCE[instance_type] keys = {} - if instance_type.startswith("n1-ultramem-") or instance_type.startswith( - "n1-megamem-" - ): + if instance_type.startswith(("n1-ultramem-", "n1-megamem-")): keys["mem"] = "memopt-ram" keys["cpu"] = "memopt-cpu" elif instance_type.startswith("n1-"): diff --git a/server/ec2spotmanager/tasks.py b/server/ec2spotmanager/tasks.py index 85820919..3b49e433 100644 --- a/server/ec2spotmanager/tasks.py +++ b/server/ec2spotmanager/tasks.py @@ -520,8 +520,7 @@ def update_instances(provider, region): # in our database, because otherwise our code here would delete it from # the database. if cloud_id in instances: - if instances[cloud_id] in instances_left: - instances_left.remove(instances[cloud_id]) + instances_left.discard(instances[cloud_id]) else: debug_not_updatable_continue.add(cloud_id) continue @@ -564,8 +563,7 @@ def update_instances(provider, region): continue instance = instances[cloud_id] - if instance in instances_left: - instances_left.remove(instance) + instances_left.discard(instance) # Check the status code and update if necessary if instance.status_code != cloud_data["status"]: diff --git a/server/ec2spotmanager/tests/test_task_graph.py b/server/ec2spotmanager/tests/test_task_graph.py index 6640fe6e..7e0c674f 100644 --- a/server/ec2spotmanager/tests/test_task_graph.py +++ b/server/ec2spotmanager/tests/test_task_graph.py @@ -279,7 +279,6 @@ def test_update_pool_graph_unsupported_running(mocker): update_requests_idx += 1 update_instances_idx += 1 cycle_and_terminate_disabled_idx += 1 - pass elif group_call[0][0]: assert group_call == call([mock_chain(), mock_chain()]) diff --git a/server/ec2spotmanager/views.py b/server/ec2spotmanager/views.py index 62b697e8..b78d3621 100644 --- a/server/ec2spotmanager/views.py +++ b/server/ec2spotmanager/views.py @@ -948,7 +948,7 @@ def retrieve(self, request, *args, **kwds): flatten = int(flatten) assert flatten in {0, 1} except (AssertionError, ValueError): - raise serializers.ValidationError("flatten must be 0 or 1") + raise serializers.ValidationError("flatten must be 0 or 1") from None serializer = self.get_serializer(self.get_object(), flatten=bool(flatten)) return Response(serializer.data) diff --git a/server/server/management/commands/manage_token_restrictions.py b/server/server/management/commands/manage_token_restrictions.py index 0f7faac4..8bf6b393 100644 --- a/server/server/management/commands/manage_token_restrictions.py +++ b/server/server/management/commands/manage_token_restrictions.py @@ -41,7 +41,7 @@ def handle(self, *args, **options): try: token = Token.objects.get(key=token_key) except Token.DoesNotExist: - raise CommandError(f"Token with key '{token_key}' not found") + raise CommandError(f"Token with key '{token_key}' not found") from None self.stdout.write(f"Found token: {token.key} (User: {token.user.username})") diff --git a/server/server/settings.py b/server/server/settings.py index 8a60f1a5..7807571c 100644 --- a/server/server/settings.py +++ b/server/server/settings.py @@ -32,8 +32,8 @@ chars = "abcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*(-_=+)" SECRET_KEY = "".join([random.choice(chars) for _ in range(64)]) f.write(SECRET_KEY) - except OSError: - raise Exception(f'Cannot open file "{SECRET_FILE}" for writing.') + except OSError as exc: + raise Exception(f'Cannot open file "{SECRET_FILE}" for writing.') from exc # SECURITY WARNING: don't run with debug turned on in production! DEBUG = True diff --git a/server/server/utils.py b/server/server/utils.py index 35a8276d..8f218c7a 100644 --- a/server/server/utils.py +++ b/server/server/utils.py @@ -109,7 +109,7 @@ def authenticate(self, request): _user, token = auth_result if not self.is_from_allowed_ip(request, token): - LOG.warning(f"IP address restricted for token: {token.key[:8]}...") + LOG.warning("IP address restricted for token: %s...", token.key[:8]) raise PermissionDenied("IP address restricted. Access denied.") return auth_result @@ -133,7 +133,7 @@ def is_from_allowed_ip(self, request, token): try: ip_obj = ip_address(client_ip) except ValueError: - LOG.warning(f"Invalid IP address format: {client_ip}") + LOG.warning("Invalid IP address format: %s", client_ip) return False # Check each restriction @@ -156,6 +156,6 @@ def is_from_allowed_ip(self, request, token): return True except ValueError: # noqa: PERF203 - LOG.warning(f"Invalid network definition: {restriction.ip_range}") + LOG.warning("Invalid network definition: %s", restriction.ip_range) return False diff --git a/server/server/views.py b/server/server/views.py index 87883542..5b62bfa7 100644 --- a/server/server/views.py +++ b/server/server/views.py @@ -97,7 +97,7 @@ def json_to_query(json_str): try: obj = json.loads(json_str, object_pairs_hook=collections.OrderedDict) except ValueError as e: - raise RuntimeError(f"Invalid JSON: {e}") + raise RuntimeError(f"Invalid JSON: {e}") from e def get_query_obj(obj, key=None): if obj is None or isinstance(obj, (str, list, int)):