Merge pull request #697 from SUNET/ylle_try-except-in-loop

Handle try-except-in-loop

requirements/fastapi_requirements.in

@@ -7,3 +7,4 @@ ndnkdf
 uvicorn[standard]
 python-multipart  # needed to parse form data
 deepdiff6
+apscheduler<4 # Next major version has API breaking changes

requirements/worker_requirements.in

@@ -1,4 +1,3 @@
 -r main.in
 -c main.txt
 jinja2
-apscheduler<4 # Next major version has API breaking changes

ruff.toml

@@ -6,5 +6,4 @@ target-version = "py310"
 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"]
+ignore = ["E501"]

src/eduid/common/logging.py

@@ -142,9 +142,9 @@ def merge_config(base_config: dict[str, Any], new_config: dict[str, Any]) -> dic
     def merge(node, key, value):
         if isinstance(value, dict):
             for item in value:
-                try:
+                if key in node:
                     merge(node[key], item, value[item])
-                except KeyError:
+                else:
                     # No such key in base_config, just set it
                     node[key] = value
         else:

src/eduid/graphdb/groupdb/db.py

@@ -56,7 +56,7 @@ def db_setup(self):
             for statement in statements:
                 try:
                     session.run(statement)
-                except ClientError as e:
+                except ClientError as e:  # noqa: PERF203
                     assert e.message is not None  # please mypy
                     acceptable_error_codes = [
                         "Neo.ClientError.Schema.ConstraintAlreadyExists",

src/eduid/scimapi/utils.py

@@ -69,7 +69,7 @@ def wrapper_run_func(*args, **kwargs):
         while True:
             try:
                 return func(*args, **kwargs)
-            except (EduIDDBError, EduIDGroupDBError, Neo4jError) as e:
+            except (EduIDDBError, EduIDGroupDBError, Neo4jError) as e:  # noqa: PERF203
                 retry += 1
                 if retry >= max_retries:
                     raise MaxRetriesReached(f"Max retries reached for {func.__name__}")

src/eduid/userdb/support/models.py

@@ -22,10 +22,8 @@ def __init__(self, data):
             pass
         elif self.add_keys:
             for key in self.add_keys:
-                try:
+                if key in _data:
                     self[key] = _data[key]
-                except KeyError:
-                    pass
         elif self.remove_keys:
             for key in self.remove_keys:
                 _data.pop(key, None)

src/eduid/userdb/user_cleaner/db.py

@@ -45,3 +45,6 @@ def get_next_user(self, cleaner_type: CleanerType) -> CleanerQueueUser | None:
         else:
             logger.debug("No document found")
             return None
+
+    def user_in_queue(self, cleaner_type: CleanerType, eppn: str) -> bool:
+        return self.db_count(spec={"cleaner_type": cleaner_type, "eduPersonPrincipalName": eppn}, limit=1) > 0

src/eduid/webapp/idp/login_context.py

@@ -301,12 +301,16 @@ def service_info(self) -> ServiceInfo | None:
 def _pick_authn_context(accrs: Sequence[str], log_tag: str) -> EduidAuthnContextClass | None:
     if len(accrs) > 1:
         logger.warning(f"{log_tag}: More than one authnContextClassRef, using the first recognised: {accrs}")
+    # make a set of implemented EduidAuthnContextClass values
+    # TODO: in python 3.12 this can be removed and test below can be 'x in EduidAuthnContextClass'
+    implemented: set[str] = {x.value for x in EduidAuthnContextClass}
+
     # first, select the ones recognised by this IdP
     known = []
     for x in accrs:
-        try:
+        if x in implemented:
             known += [EduidAuthnContextClass(x)]
-        except ValueError:
+        else:
             logger.info(f"Unknown authnContextClassRef: {x}")
             known += [EduidAuthnContextClass.NOT_IMPLEMENTED]
     if not known:

src/eduid/workers/job_runner/jobs/skv.py

@@ -1,7 +1,6 @@
 from eduid.common.misc.timeutil import utc_now
 from eduid.common.rpc.exceptions import MsgTaskFailed
 from eduid.common.rpc.msg_relay import DeregisteredCauseCode, NavetData
-from eduid.userdb.exceptions import UserDoesNotExist
 from eduid.userdb.meta import CleanerType
 from eduid.userdb.user import User
 from eduid.userdb.user_cleaner.db import CleanerQueueUser
@@ -18,10 +17,10 @@ def gather_skv_users(context: Context):
     users: list[User] = context.db.get_unterminated_users_with_nin()
     context.logger.debug(f"gathered {len(users)} users to check")
     for user in users:
-        try:
+        if context.cleaner_queue.user_in_queue(cleaner_type=CleanerType.SKV, eppn=user.eppn):
             context.cleaner_queue.get_user_by_eppn(user.eppn)
             context.logger.debug(f"{user.eppn} already in queue")
-        except UserDoesNotExist:
+        else:
             queue_user: CleanerQueueUser = CleanerQueueUser(
                 eppn=user.eppn, cleaner_type=CleanerType.SKV, identities=user.identities
             )