From 0b534b8da5680c6176053659974f67ae659503ed Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 19 Sep 2024 11:05:31 +0000 Subject: [PATCH 1/6] re-enable PERF203 --- ruff.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ruff.toml b/ruff.toml index 518e55bb2..fcff9c805 100644 --- a/ruff.toml +++ b/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"] From b5e4a9b65b233fa39387aadc97532d31ca945db5 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 19 Sep 2024 11:06:50 +0000 Subject: [PATCH 2/6] do not use KeyError to check if values exist --- src/eduid/common/logging.py | 4 ++-- src/eduid/userdb/support/models.py | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/eduid/common/logging.py b/src/eduid/common/logging.py index a0ae99b39..84948ac5b 100644 --- a/src/eduid/common/logging.py +++ b/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: diff --git a/src/eduid/userdb/support/models.py b/src/eduid/userdb/support/models.py index ed5158eab..a88f585d7 100644 --- a/src/eduid/userdb/support/models.py +++ b/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) From 4e565625ab72bd9dcbb9b8d7c8b8325637637764 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 19 Sep 2024 11:08:25 +0000 Subject: [PATCH 3/6] use db_count to check if user exists in database --- src/eduid/userdb/user_cleaner/db.py | 3 +++ src/eduid/workers/job_runner/jobs/skv.py | 5 ++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/eduid/userdb/user_cleaner/db.py b/src/eduid/userdb/user_cleaner/db.py index fe06f1091..570c47667 100644 --- a/src/eduid/userdb/user_cleaner/db.py +++ b/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 diff --git a/src/eduid/workers/job_runner/jobs/skv.py b/src/eduid/workers/job_runner/jobs/skv.py index cf95e9252..62dcab2f4 100644 --- a/src/eduid/workers/job_runner/jobs/skv.py +++ b/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 ) From fb52a7e5f38699496c600af38b95c0e226db79ce Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 19 Sep 2024 11:10:15 +0000 Subject: [PATCH 4/6] move apscheduler requirement to correct file --- requirements/fastapi_requirements.in | 1 + requirements/worker_requirements.in | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/fastapi_requirements.in b/requirements/fastapi_requirements.in index 7b1c6eb19..e029aa51e 100644 --- a/requirements/fastapi_requirements.in +++ b/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 diff --git a/requirements/worker_requirements.in b/requirements/worker_requirements.in index 0f09ca02b..86ce6b370 100644 --- a/requirements/worker_requirements.in +++ b/requirements/worker_requirements.in @@ -1,4 +1,3 @@ -r main.in -c main.txt jinja2 -apscheduler<4 # Next major version has API breaking changes \ No newline at end of file From 1baf66214f55204feaff10852ee9cd7465d99517 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 19 Sep 2024 11:29:48 +0000 Subject: [PATCH 5/6] use workaround for current python to remove try-except --- src/eduid/webapp/idp/login_context.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/eduid/webapp/idp/login_context.py b/src/eduid/webapp/idp/login_context.py index f1c817fe2..b63847a7e 100644 --- a/src/eduid/webapp/idp/login_context.py +++ b/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: From e80c91000435c3d73e8241ad2fcdda5affc271da Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 19 Sep 2024 11:30:11 +0000 Subject: [PATCH 6/6] leave actual error handling as is --- src/eduid/graphdb/groupdb/db.py | 2 +- src/eduid/scimapi/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/eduid/graphdb/groupdb/db.py b/src/eduid/graphdb/groupdb/db.py index 5d754c63d..c38cf69a3 100644 --- a/src/eduid/graphdb/groupdb/db.py +++ b/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", diff --git a/src/eduid/scimapi/utils.py b/src/eduid/scimapi/utils.py index 14a04497e..0f04ec0a2 100644 --- a/src/eduid/scimapi/utils.py +++ b/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__}")