Skip to content

Commit

Permalink
Merge pull request #696 from SUNET/ylle-new-linting
Browse files Browse the repository at this point in the history
add more linting rules and fixes
  • Loading branch information
johanlundberg authored Sep 19, 2024
2 parents 6a379f5 + 597119a commit 14f017a
Show file tree
Hide file tree
Showing 14 changed files with 36 additions and 39 deletions.
6 changes: 5 additions & 1 deletion ruff.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@ line-length = 120
target-version = "py310"

[lint]
select = ["E4", "E7", "E9", "F", "W", "I", "ASYNC", "UP"]
select = ["E", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF", "FURB"]

# For now, ignore E501 (line too long) errors. They are in strings and comments.
# For now, ignore PERF203 (try-except-in-loop), TODO: fix them.
ignore = ["E501", "PERF203"]
12 changes: 8 additions & 4 deletions src/eduid/graphdb/groupdb/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,10 @@ def get_groups_by_property(self, key: str, value: str, skip=0, limit=100):
RETURN g as group SKIP $skip LIMIT $limit
"""
with self.db.driver.session(default_access_mode=READ_ACCESS) as session:
for record in session.run(q, scope=self.scope, value=value, skip=skip, limit=limit):
res.append(self._load_group(record.data()["group"]))
res = [
self._load_group(record.data()["group"])
for record in session.run(q, scope=self.scope, value=value, skip=skip, limit=limit)
]
return res

def get_groups(self, skip: int = 0, limit: int = 100) -> list[Group]:
Expand All @@ -295,8 +297,10 @@ def get_groups(self, skip: int = 0, limit: int = 100) -> list[Group]:
RETURN g as group SKIP $skip LIMIT $limit
"""
with self.db.driver.session(default_access_mode=READ_ACCESS) as session:
for record in session.run(q, scope=self.scope, skip=skip, limit=limit):
res.append(self._load_group(record.data()["group"]))
res = [
self._load_group(record.data()["group"])
for record in session.run(q, scope=self.scope, skip=skip, limit=limit)
]
return res

def _get_groups_for_role(self, label: Label, identifier: str, role: Role):
Expand Down
9 changes: 3 additions & 6 deletions src/eduid/scimapi/routers/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@
@groups_router.get("/", response_model=ListResponse)
async def on_get_all(req: ContextRequest) -> ListResponse:
db_groups = req.context.groupdb.get_groups()
resources = []
for db_group in db_groups:
resources.append({"id": str(db_group.scim_id), "displayName": db_group.graph.display_name})
resources = [{"id": str(db_group.scim_id), "displayName": db_group.graph.display_name} for db_group in db_groups]
return ListResponse(total_results=len(db_groups), resources=resources)


Expand Down Expand Up @@ -302,7 +300,6 @@ async def search(req: ContextRequest, query: SearchRequest) -> ListResponse:
else:
raise BadRequest(scim_type="invalidFilter", detail=f"Can't filter on attribute {_filter.attr}")

resources = []
for this in groups:
resources.append({"id": str(this.scim_id), "displayName": this.display_name})
resources = [{"id": str(this.scim_id), "displayName": this.display_name} for this in groups]

return ListResponse(total_results=total_count, resources=resources)
9 changes: 4 additions & 5 deletions src/eduid/scimapi/test-scripts/scim-util.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,10 @@ def put_group(api: Api, scim_id: str, data: dict[str, Any], token: str | None =
if display_name:
scim["displayName"] = display_name
if members:
new_members = []
for member in members:
new_members.append(
{"$ref": f'{api}/Users/{member["id"]}', "value": member["id"], "display": member["display_name"]}
)
new_members = [
{"$ref": f'{api}/Users/{member["id"]}', "value": member["id"], "display": member["display_name"]}
for member in members
]
scim["members"] = new_members
if "data" in data:
if NUTID_GROUP_V1 not in scim["schemas"]:
Expand Down
11 changes: 5 additions & 6 deletions src/eduid/userdb/group_management/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,11 @@ def get_states_by_email_addresses(self, email_addresses: list[str]) -> list[Grou
:raise self.DocumentDoesNotExist: No document match the search criteria
"""
states: list[GroupInviteState] = []
for email_address in email_addresses:
spec = {"email_address": email_address}
for this in self._get_documents_by_filter(spec):
states.append(GroupInviteState.from_dict(this))

states: list[GroupInviteState] = [
GroupInviteState.from_dict(state)
for email_address in email_addresses
for state in self._get_documents_by_filter({"email_address": email_address})
]
return states

def save(self, state: GroupInviteState, is_in_database: bool) -> SaveResult:
Expand Down
4 changes: 1 addition & 3 deletions src/eduid/webapp/common/api/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ def get_marked_given_name(given_name: str, given_name_marking: str | None) -> st
else:
_given_names.append(name)

_optional_marked_names: list[str | None] = []
for i in given_name_marking:
_optional_marked_names.append(_given_names[int(i)])
_optional_marked_names: list[str | None] = [_given_names[int(i)] for i in given_name_marking]
# remove None values
# i.e. 0 index and hyphenated names second part placeholder
_marked_names: list[str] = [name for name in _optional_marked_names if name is not None]
Expand Down
2 changes: 1 addition & 1 deletion src/eduid/webapp/common/api/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ def _check_api_response(
def _assure_not_in_dict(d: Mapping[str, Any], unwanted_key: str):
assert unwanted_key not in d, f"Key {unwanted_key} should not be in payload, but it is: {payload}"
v2: Mapping[str, Any]
for _k2, v2 in d.items():
for v2 in d.values():
if isinstance(v2, dict):
_assure_not_in_dict(v2, unwanted_key)

Expand Down
3 changes: 1 addition & 2 deletions src/eduid/webapp/common/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,7 @@ def get_zxcvbn_terms(user: User) -> list[str]:

# Mail addresses
if user.mail_addresses.count:
for item in user.mail_addresses.to_list():
user_input.append(item.email.split("@")[0])
user_input.extend(item.email.split("@")[0] for item in user.mail_addresses.to_list())

return user_input

Expand Down
2 changes: 1 addition & 1 deletion src/eduid/webapp/common/api/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def is_valid_password(password: str, user_info: Sequence[str], min_entropy: int,
# Check password complexity with zxcvbn
result = zxcvbn(password, user_inputs=user_info)
_guesses = result.get("guesses", 1)
_pw_entropy = math.log(_guesses, 2)
_pw_entropy = math.log2(_guesses)
if _pw_entropy < min_entropy:
raise ValueError("The password complexity is too weak.")
# This is the SWAMID requirement for zxcvbn since 2021:
Expand Down
2 changes: 1 addition & 1 deletion src/eduid/webapp/common/authn/fido_tokens.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def _get_fido2server(credentials: dict[ElementKey, FidoCred], fido2rp: PublicKey
# (assume all app-ids are the same - authenticating with a mix of different
# app-ids isn't supported in current Webauthn)
app_id = None
for _k, v in credentials.items():
for v in credentials.values():
if v.app_id:
app_id = v.app_id
break
Expand Down
2 changes: 1 addition & 1 deletion src/eduid/webapp/common/authn/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def get_saml_attribute(session_info: SessionInfo, attr_name: str) -> list[str] |
logger.debug(f"SAML attributes received: {attributes}")

# Look for the canonicalized attribute in the SAML assertion attributes
for saml_attr, _ in attributes.items():
for saml_attr in attributes.keys():
if saml_attr.lower() == attr_name.lower():
return attributes[saml_attr]
return None
Expand Down
7 changes: 3 additions & 4 deletions src/eduid/webapp/security/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,9 @@ def get_approved_security_keys() -> dict[str, Any]:
)
parsed_entries.append(authenticator_info)

approved_keys_list: list[str] = []
for entry in parsed_entries:
if entry.description and is_authenticator_mfa_approved(entry):
approved_keys_list.append(entry.description)
approved_keys_list: list[str] = [
entry.description for entry in parsed_entries if entry.description and is_authenticator_mfa_approved(entry)
]

# remove case-insensitive duplicates from list, while maintaining the original case
marker = set()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,7 @@ def _search_by_SSNo(self, national_identity_number: str) -> list[str]:
if record is None:
return []

mobile_numbers = []
for r in record:
mobile_numbers.append(r.Mobiles)
mobile_numbers = [r.Mobiles for r in record]

return mobile_numbers

Expand Down
2 changes: 1 addition & 1 deletion src/eduid/workers/msg/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def load_template(template_dir: str, filename: str, message_dict: Mapping[str, s

if os.path.isdir(template_dir):
try:
f = ".".join([filename, lang])
f = f"{filename}.{lang}"
if os.path.exists(os.path.join(template_dir, f)):
filename = f
template = Environment(loader=FileSystemLoader(template_dir)).get_template(filename)
Expand Down

0 comments on commit 14f017a

Please sign in to comment.