Skip to content

Commit

Permalink
Clean up SIM*. SLF*, T*, TRY*, UP006
Browse files Browse the repository at this point in the history
  • Loading branch information
wsanchez committed Nov 26, 2024
1 parent f9eaeb4 commit ddbd129
Show file tree
Hide file tree
Showing 25 changed files with 102 additions and 96 deletions.
6 changes: 0 additions & 6 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,12 @@
# add these directories to sys.path here. If the directory is relative to the
# documentation root, use os.path.abspath to make it absolute, like shown here.

import sys
from os import environ
from os.path import abspath, join
from pprint import pprint
from sys import path


path.insert(0, abspath(join("..", "src")))

pprint(environ)
pprint(sys.path)


# -- Project information -----------------------------------------------------

Expand Down
21 changes: 5 additions & 16 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ ignore = [
"RSE102", # Unnecessary parentheses on raised exception
"S101", # Use of `assert` detected
"S608", # Possible SQL injection vector through string-based query construction
"SIM108", # Use ternary operator
"TD001", # Invalid TODO tag: `FIXME`
"TD002", # Missing author in TODO
"TRY003", # Avoid specifying long messages outside the exception class
"TRY400", # Use `logging.exception` instead of `logging.error`

# Reconsider
"TID252", # Prefer absolute imports over relative imports from parent modules
Expand All @@ -130,24 +135,8 @@ ignore = [
"FBT001", # Boolean-typed positional argument in function definition
"FBT002", # Boolean default positional argument in function definition
"FBT003", # Boolean positional value in function call
"SIM102", # Use a single `if` statement instead of nested `if` statements
"SIM105", # Use `contextlib.suppress(NoSectionError, NoOptionError)` instead of `try`-`except`-`pass`
"SIM108", # Use ternary operator
"SIM114", # Combine `if` branches using logical `or` operator
"SIM115", # Use a context manager for opening files
"SIM118", # Use `key in dict` instead of `key in dict.keys()`
"SLF001", # Private member accessed
"T201", # `print` found
"T203", # `pprint` found
"TC001", # Move application import into a type-checking block
"TC003", # Move standard library import into a type-checking block
"TD001", # Invalid TODO tag: `FIXME`
"TD002", # Missing author in TODO
"TD003", # Missing issue link on the line following this TODO
"TRY003", # Avoid specifying long messages outside the exception class
"TRY301", # Abstract `raise` to an inner function
"TRY400", # Use `logging.exception` instead of `logging.error`
"UP006", # Use `collections.deque` instead of `Deque` for type annotation
"UP007", # Use `X | Y` for type annotations
"UP017", # Use `datetime.UTC` alias
"UP031", # Use format specifiers instead of percent format
Expand Down
2 changes: 1 addition & 1 deletion src/ims/application/_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,7 @@ async def editStreetsResource(
except JSONDecodeError as e:
return invalidJSONResponse(request, e)

for eventID in edits.keys():
for eventID in edits.keys(): # noqa: SIM118
existing = await store.concentricStreets(eventID)

for _streetID, _streetName in existing.items():
Expand Down
4 changes: 2 additions & 2 deletions src/ims/application/_eventsource.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from collections import deque
from collections.abc import Mapping
from time import time
from typing import Any, ClassVar, Deque
from typing import Any, ClassVar

from attrs import field, frozen
from twisted.logger import ILogObserver, Logger
Expand Down Expand Up @@ -76,7 +76,7 @@ class DataStoreEventSourceLogObserver:
_log: ClassVar[Logger] = Logger()

_listeners: list[IRequest] = field(init=False, factory=list)
_events: Deque[tuple[int, Event]] = field(
_events: deque[tuple[int, Event]] = field(
init=False, factory=lambda: deque(maxlen=1000)
)
_start: float = field(init=False, factory=time)
Expand Down
18 changes: 10 additions & 8 deletions src/ims/application/_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,11 @@ class HTTPPageGetter(http.HTTPClient):
def connectionMade(self):
method = _ensureValidMethod(getattr(self.factory, "method", b"GET"))
self.sendCommand(method, _ensureValidURI(self.factory.path))
if self.factory.scheme == b"http" and self.factory.port != 80: # noqa: PLR2004
host = b"%b:%d" % (self.factory.host, self.factory.port)
elif self.factory.scheme == b"https" and self.factory.port != 443: # noqa: PLR2004
if (
self.factory.scheme == b"http" and self.factory.port != 80 # noqa: PLR2004
) or (
self.factory.scheme == b"https" and self.factory.port != 443 # noqa: PLR2004
):
host = b"%b:%d" % (self.factory.host, self.factory.port)
else:
host = self.factory.host
Expand Down Expand Up @@ -169,8 +171,8 @@ def handleStatus_301(self):
return
url = l[0]
if self.followRedirect:
self.factory._redirectCount += 1
if self.factory._redirectCount >= self.factory.redirectLimit:
self.factory._redirectCount += 1 # noqa: SLF001
if self.factory._redirectCount >= self.factory.redirectLimit: # noqa: SLF001
err = error.InfiniteRedirection(
self.status, b"Infinite redirection detected", location=url
)
Expand Down Expand Up @@ -230,7 +232,7 @@ def connectionLost(self, reason):
# Only if we think we're completely done do we tell the factory that
# we're "disconnected". This way when we're following redirects,
# only the last protocol used will fire the _disconnectedDeferred.
self.factory._disconnectedDeferred.callback(None)
self.factory._disconnectedDeferred.callback(None) # noqa: SLF001

def handleResponse(self, response):
if self.quietLoss:
Expand Down Expand Up @@ -548,10 +550,10 @@ def gotHeaders(self, headers):

def openFile(self, partialContent):
if partialContent:
file = open(self.fileName, "rb+")
file = open(self.fileName, "rb+") # noqa: SIM115
file.seek(0, 2)
else:
file = open(self.fileName, "wb")
file = open(self.fileName, "wb") # noqa: SIM115
return file

def pageStart(self, partialContent):
Expand Down
5 changes: 2 additions & 3 deletions src/ims/application/_klein.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,9 +439,8 @@ def notAuthenticatedError(
Not authenticated.
"""
requestedWith = request.getHeader("X-Requested-With")
if requestedWith is not None:
if requestedWith == "XMLHttpRequest":
return forbiddenResponse(request)
if requestedWith == "XMLHttpRequest":
return forbiddenResponse(request)

element = redirect(request, URLs.login, origin="o")
return renderElement( # type: ignore[return-value]
Expand Down
4 changes: 2 additions & 2 deletions src/ims/auth/_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def fromText(cls, tokenText: str, *, key: JSONWebKey) -> "JSONWebToken":
Create a token from text.
"""
try:
jwt = JWT(jwt=tokenText, key=key._jwk)
jwt = JWT(jwt=tokenText, key=key._jwk) # noqa: SLF001
except InvalidJWSSignature as e:
raise InvalidCredentialsError("Invalid JWT token") from e

Expand All @@ -206,7 +206,7 @@ def fromClaims(
Create a token from text.
"""
jwt = JWT(header={"typ": "JWT", "alg": "HS256"}, claims=claims.asJSON())
jwt.make_signed_token(key._jwk)
jwt.make_signed_token(key._jwk) # noqa: SLF001

return cls(jwt=jwt)

Expand Down
2 changes: 2 additions & 0 deletions src/ims/auth/test/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
# limitations under the License.
##

# ruff: noqa: SLF001

"""
Tests for L{ims.auth._provider}.
"""
Expand Down
5 changes: 2 additions & 3 deletions src/ims/config/_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from collections.abc import Callable, Sequence
from configparser import ConfigParser, NoOptionError, NoSectionError
from contextlib import suppress
from datetime import timedelta as TimeDelta
from functools import partial
from os import environ
Expand Down Expand Up @@ -110,10 +111,8 @@ def valueFromConfig(
value = environ.get(f"IMS_{variable}")

if not value:
try:
with suppress(NoSectionError, NoOptionError):
value = self._configParser.get(section, option)
except (NoSectionError, NoOptionError):
pass

if value:
return value
Expand Down
2 changes: 2 additions & 0 deletions src/ims/config/test/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
# limitations under the License.
##

# ruff: noqa: SLF001

"""
Tests for L{ims.config._config}.
"""
Expand Down
48 changes: 24 additions & 24 deletions src/ims/directory/clubhouse_db/_dms.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ class _State:
Internal mutable state for :class:`Configuration`.
"""

_personnel: Iterable[Ranger] = field(default=(), init=False)
_positions: Iterable[Position] = field(default=(), init=False)
_personnelLastUpdated: float = field(default=0.0, init=False)
_dbpool: adbapi.ConnectionPool | None = field(default=None, init=False)
_busy: bool = field(default=False, init=False)
_dbErrorCount: int = field(default=0, init=False)
personnel: Iterable[Ranger] = field(default=(), init=False)
positions: Iterable[Position] = field(default=(), init=False)
personnelLastUpdated: float = field(default=0.0, init=False)
dbpool: adbapi.ConnectionPool | None = field(default=None, init=False)
busy: bool = field(default=False, init=False)
dbErrorCount: int = field(default=0, init=False)

host: str
database: str
Expand All @@ -100,7 +100,7 @@ def dbpool(self) -> adbapi.ConnectionPool:
"""
Set up a database pool if needed and return it.
"""
if self._state._dbpool is None:
if self._state.dbpool is None:
if (
not self.host
and not self.database
Expand All @@ -123,9 +123,9 @@ def dbpool(self) -> adbapi.ConnectionPool:
if dbpool is None:
raise DatabaseError("Unable to set up database pool.")

self._state._dbpool = dbpool
self._state.dbpool = dbpool

return self._state._dbpool
return self._state.dbpool

async def _queryPositionsByID(self) -> Mapping[str, Position]:
self._log.info("Retrieving positions from Duty Management System...")
Expand Down Expand Up @@ -194,19 +194,19 @@ async def positions(self) -> Iterable[Position]:
Look up all positions.
"""
# Call self.personnel() to make sure we have current data, then return
# self._state._positions, which will have been set.
# self._state.positions, which will have been set.
await self.personnel()
return self._state._positions
return self._state.positions

async def personnel(self) -> Iterable[Ranger]:
"""
Look up all personnel.
"""
now = time()
elapsed = now - self._state._personnelLastUpdated
elapsed = now - self._state.personnelLastUpdated

if not self._state._busy and elapsed > self.cacheInterval:
self._state._busy = True
if not self._state.busy and elapsed > self.cacheInterval:
self._state.busy = True
try:
try:
rangersByID = await self._queryRangersByID()
Expand All @@ -222,18 +222,18 @@ async def personnel(self) -> Iterable[Ranger]:
continue
position.members.add(ranger)

self._state._personnel = tuple(rangersByID.values())
self._state._positions = tuple(positionsByID.values())
self._state._personnelLastUpdated = time()
self._state._dbErrorCount = 0
self._state.personnel = tuple(rangersByID.values())
self._state.positions = tuple(positionsByID.values())
self._state.personnelLastUpdated = time()
self._state.dbErrorCount = 0

except Exception as e:
self._state._personnelLastUpdated = 0
self._state._dbpool = None
self._state._dbErrorCount += 1
self._state.personnelLastUpdated = 0
self._state.dbpool = None
self._state.dbErrorCount += 1

if isinstance(e, (SQLDatabaseError, SQLOperationalError)):
if self._state._dbErrorCount < 2: # noqa: PLR2004
if self._state.dbErrorCount < 2: # noqa: PLR2004
self._log.info(
"Retrying loading personnel from DMS "
"after error: {error}",
Expand All @@ -257,10 +257,10 @@ async def personnel(self) -> Iterable[Ranger]:
) from e

finally:
self._state._busy = False
self._state.busy = False

try:
return self._state._personnel
return self._state.personnel
except AttributeError:
raise DMSError("No personnel data loaded.") from None

Expand Down
7 changes: 5 additions & 2 deletions src/ims/directory/clubhouse_db/test/dummy.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
Mock objects for Clubhouse directory.
"""

from collections.abc import MutableSequence
from typing import Any, cast
from typing import TYPE_CHECKING, Any, cast

from twisted.internet.defer import Deferred, fail, succeed

Expand All @@ -28,6 +27,10 @@
from ..._directory import hashPassword


if TYPE_CHECKING:
from collections.abc import MutableSequence


__all__ = ()


Expand Down
2 changes: 2 additions & 0 deletions src/ims/directory/clubhouse_db/test/test_dms.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
# limitations under the License.
##

# ruff: noqa: SLF001

"""
Tests for L{ims.directory.clubhouse_db._dms}.
"""
Expand Down
2 changes: 2 additions & 0 deletions src/ims/directory/file/test/test_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
# limitations under the License.
##

# ruff: noqa: SLF001

"""
Tests for L{ims.directory.file._directory}.
"""
Expand Down
4 changes: 2 additions & 2 deletions src/ims/ext/sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def executeAndPrint(self, sql: str, parameters: Parameters | None = None) -> Non
"""

def emit(row: Iterable[object]) -> None:
print(" | ".join(str(i) for i in row))
print(" | ".join(str(i) for i in row)) # noqa: T201

printHeader = True

Expand Down Expand Up @@ -158,7 +158,7 @@ def validateForeignKeys(self) -> None:
rowid=rowid,
referred=referred,
constraint=constraint,
row={k: row[k] for k in row.keys()},
row={k: row[k] for k in row.keys()}, # noqa: SIM118
)

valid = False
Expand Down
2 changes: 2 additions & 0 deletions src/ims/ext/test/test_sqlite.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# ruff: noqa: SLF001

"""
Tests for :mod:`ranger-ims-server.ext.sqlite`
"""
Expand Down
13 changes: 5 additions & 8 deletions src/ims/model/_address.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def _cmp(self, other: Any, methodName: str) -> bool:
if other is None:
return self.description is None

return ComparisonMixIn._cmp(self, other, methodName)
return ComparisonMixIn._cmp(self, other, methodName) # noqa: SLF001


@frozen(kw_only=True, eq=False)
Expand Down Expand Up @@ -98,14 +98,11 @@ def _cmp(self, other: Any, methodName: str) -> bool:
if other is None:
return self._allNone()

if other.__class__ is TextOnlyAddress:
if self._allNone():
method = cast(
Callable[[str], bool], getattr(self.description, methodName)
)
return method(other.description)
if other.__class__ is TextOnlyAddress and self._allNone():
method = cast(Callable[[str], bool], getattr(self.description, methodName))
return method(other.description)

return ComparisonMixIn._cmp(self, other, methodName)
return ComparisonMixIn._cmp(self, other, methodName) # noqa: SLF001

def __hash__(self) -> int:
if self._allNone():
Expand Down
Loading

0 comments on commit ddbd129

Please sign in to comment.